Skip to content

Commit

Permalink
Registry: Protect against putObject misuse, handle duplicate registra…
Browse files Browse the repository at this point in the history 
…tions better
  • Loading branch information
@sfPlayer1
sfPlayer1 committed Apr 6, 2014
1 parent c8a0b72 commit 8e44006
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 46 deletions.
7 changes: 7 additions & 0 deletions src/main/java/cpw/mods/fml/common/FMLLog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ public static void severe(String format, Object... data)
log(Level.ERROR, format, data);
}

public static void bigWarning(String format, Object... data)
{
log(Level.WARN, "****************************************");
log(Level.WARN, "* "+format, data);
log(Level.WARN, "****************************************");
}

public static void warning(String format, Object... data)
{
log(Level.WARN, format, data);
Expand Down
76 changes: 61 additions & 15 deletions src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void set(FMLControlledNamespacedRegistry<I> registry)

for (I thing : (Iterable<I>) registry)
{
super.addObject(registry.getId(thing), registry.getNameForObject(thing), thing);
addObjectRaw(registry.getId(thing), registry.getNameForObject(thing), thing);
}
}

Expand All @@ -70,6 +70,39 @@ public void addObject(int id, String name, Object thing)
GameData.getMain().register(thing, name, id);
}

/**
* DANGEROUS! EVIL! DO NOT USE!
*
* @deprecated register through {@link GameRegistry} instead.
*/
@Override
@Deprecated
public void putObject(Object objName, Object obj)
{
String name = (String) objName;
I thing = (I) obj;

if (name == null) throw new NullPointerException("Can't use a null-name for the registry.");
if (name.isEmpty()) throw new IllegalArgumentException("Can't use an empty name for the registry.");
if (thing == null) throw new NullPointerException("Can't add null-object to the registry.");

String existingName = getNameForObject(thing);

if (existingName == null)
{
FMLLog.bigWarning("Ignoring putObject(%s, %s), not resolvable", name, thing);
}
else if (existingName.equals(name))
{
FMLLog.bigWarning("Ignoring putObject(%s, %s), already added", name, thing);
}
else
{
FMLLog.bigWarning("Ignoring putObject(%s, %s), adding alias to %s instead", name, thing, existingName);
addAlias(name, existingName);
}
}

@Override
public I getObject(String name)
{
Expand Down Expand Up @@ -235,33 +268,34 @@ int add(int id, String name, I thing, BitSet availabilityMap)
name = prefix + ":"+ name;
}

if (getRaw(name) != null)
if (getRaw(name) == thing) // already registered, return prev registration's id
{
FMLLog.bigWarning("The object %s has been registered twice for the same name %s.", thing, name);
return getId(thing);
}
if (getRaw(name) != null) // duplicate name, will crash later due to the BiMap
{
FMLLog.warning("****************************************");
FMLLog.warning("* The name %s has been registered twice, for %s and %s.", name, getRaw(name), thing);
FMLLog.warning("****************************************");
FMLLog.bigWarning("The name %s has been registered twice, for %s and %s.", name, getRaw(name), thing);
}
if (getId(thing) >= 0)
if (getId(thing) >= 0) // duplicate object, will crash later due to the BiMap
{
FMLLog.warning("****************************************");
FMLLog.warning("* The object %s has been registered twice, using the names %s and %s.", thing, getNameForObject(thing), name);
FMLLog.warning("****************************************");
FMLLog.bigWarning("The object %s has been registered twice, using the names %s and %s.", thing, getNameForObject(thing), name);
}
if (GameData.isFrozen(this))
{
FMLLog.warning("****************************************");
FMLLog.warning("* The object %s (name %s) is being added too late.", thing, name);
FMLLog.warning("****************************************");
FMLLog.bigWarning("The object %s (name %s) is being added too late.", thing, name);
}

super.addObject(idToUse, name, thing);
FMLLog.finer("Add : %s %d %s", name, idToUse, thing);
addObjectRaw(idToUse, name, thing);

FMLLog.finer("Registry add: %s %d %s", name, idToUse, thing);
return idToUse;
}

void addAlias(String from, String to)
{
aliases.put(from, to);
FMLLog.finer("Registry alias: %s -> %s", from, to);
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -293,7 +327,19 @@ void dump()
for (int id : ids)
{
I thing = getRaw(id);
FMLLog.finer("Registry : %s %d %s", getNameForObject(thing), id, thing);
FMLLog.finer("Registry: %s %d %s", getNameForObject(thing), id, thing);
}
}

/**
* Version of addObject not using the API restricting overrides.
*/
private void addObjectRaw(int id, String name, I thing)
{
if (name == null) throw new NullPointerException();
if (thing == null) throw new NullPointerException();

underlyingIntegerMap.func_148746_a(thing, id); // obj <-> id
super.putObject(ensureNamespaced(name), thing); // name <-> obj
}
}
59 changes: 28 additions & 31 deletions src/main/java/cpw/mods/fml/common/registry/GameData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -740,10 +740,7 @@ int registerItem(Item item, String name, String modId, int idHint)
throw new IllegalStateException(String.format("The Item Registry slot %d is already used by %s", idHint, iItemRegistry.getObjectById(idHint)));
}

if (!freeSlot(idHint)) // temporarily free the slot occupied by the Block for the ItemBlock registration
{
throw new IllegalStateException(String.format("The Registry slot %d is supposed to be blocked by the ItemBlock's Block's blockId at this point.", idHint));
}
freeSlot(idHint); // temporarily free the slot occupied by the Block for the item registration
}

int itemId = iItemRegistry.add(idHint, name, item, availabilityMap);
Expand All @@ -756,11 +753,8 @@ int registerItem(Item item, String name, String modId, int idHint)
}
}

// normal item, block the Block Registry slot with the same id
if (useSlot(itemId))
{
throw new IllegalStateException(String.format("Registry slot %d is supposed to be empty when adding a non-ItemBlock with the same id.", itemId));
}
// block the Block Registry slot with the same id
useSlot(itemId);

return itemId;
}
Expand All @@ -779,10 +773,7 @@ int registerBlock(Block block, String name, String modId, int idHint)
}
int blockId = iBlockRegistry.add(idHint, name, block, availabilityMap);

if (useSlot(blockId))
{
throw new IllegalStateException(String.format("Registry slot %d is supposed to be empty when adding a Block with the same id.", blockId));
}
useSlot(blockId);

return blockId;
}
Expand All @@ -796,18 +787,14 @@ private void block(int id)
useSlot(id);
}

private boolean useSlot(int id)
private void useSlot(int id)
{
boolean oldValue = availabilityMap.get(id);
availabilityMap.set(id);
return oldValue;
}

private boolean freeSlot(int id)
private void freeSlot(int id)
{
boolean oldValue = availabilityMap.get(id);
availabilityMap.clear(id);
return oldValue;
}

@SuppressWarnings("unchecked")
Expand All @@ -825,14 +812,19 @@ private void testConsistency() {
for (Block block : (Iterable<Block>) iBlockRegistry)
{
int id = iBlockRegistry.getId(block);
String name = iBlockRegistry.getNameForObject(block);

if (!availabilityMap.get(id))
{
throw new IllegalStateException(String.format("Registry entry for block %s, id %d, marked as empty.", block, id));
if (id < 0) {
throw new IllegalStateException(String.format("Registry entry for block %s, name %s, doesn't yield an id", block, name));
}
if (blockedIds.contains(id))
{
throw new IllegalStateException(String.format("Registry entry for block %s, id %d, marked as dangling.", block, id));
if (name == null) {
throw new IllegalStateException(String.format("Registry entry for block %s, id %d, doesn't yield a name", block, id));
}
if (!availabilityMap.get(id)) {
throw new IllegalStateException(String.format("Registry entry for block %s, id %d, name %s, marked as empty.", block, id, name));
}
if (blockedIds.contains(id)) {
throw new IllegalStateException(String.format("Registry entry for block %s, id %d, name %s, marked as dangling.", block, id, name));
}
}

Expand All @@ -841,14 +833,19 @@ private void testConsistency() {
for (Item item : (Iterable<Item>) iItemRegistry)
{
int id = iItemRegistry.getId(item);
String name = iItemRegistry.getNameForObject(item);

if (!availabilityMap.get(id))
{
throw new IllegalStateException(String.format("Registry entry for item %s, id %d, marked as empty.", item, id));
if (id < 0) {
throw new IllegalStateException(String.format("Registry entry for item %s, name %s, doesn't yield an id", item, name));
}
if (blockedIds.contains(id))
{
throw new IllegalStateException(String.format("Registry entry for item %s, id %d, marked as dangling.", item, id));
if (name == null) {
throw new IllegalStateException(String.format("Registry entry for item %s, id %d, doesn't yield a name", item, id));
}
if (!availabilityMap.get(id)) {
throw new IllegalStateException(String.format("Registry entry for item %s, id %d, name %s, marked as empty.", item, id, name));
}
if (blockedIds.contains(id)) {
throw new IllegalStateException(String.format("Registry entry for item %s, id %d, name %s, marked as dangling.", item, id, name));
}

if (item instanceof ItemBlock)
Expand Down

0 comments on commit 8e44006

Please sign in to comment.