Substitution aliases severely broken

[marvin-roesch]

It appears that substitution aliases for blocks are broken quite substantially. Substituting any block for another one will result in various issues at runtime.

Example (code is executed in pre-init phase):

Block block = new FurnaceSubstitution();
Item item = new ItemBlock(block);
GameRegistry.addSubstitutionAlias("minecraft:furnace", GameRegistry.Type.BLOCK, block);
GameRegistry.addSubstitutionAlias("minecraft:furnace", GameRegistry.Type.ITEM, item);

The issues I've noticed so far are as follows:

1. Block breaking does not spawn particles or play a sound
2. Substituted blocks are not saved when leaving the world, resulting in them being gone upon joining again

I haven't tracked down the part of the registry system where the issue would be fixed, but I think I found the underlying issue: Substitution does not modify RegistryNamespaced.underlyingIntegerMap properly or at all. I've traced this down by putting a breakpoint in the 2001 case for RenderGlobal.playAuxSFX and stepping into Block.getBlockById. All the way down the chain, I found that the underlying map returns -1 for the block which will then default to an air block. Hence, the game thinks the block is air and consequently does not save it on exit properly as well.

I have only tried this on the latest Forge for 1.9 (12.16.1.1889), but I assume it was an issue before the new registry system as well since subsitution was not really affected by that.

Any input from other modders working with substitutions would be appreciated, maybe we'll be able to track down the bug in the substitution system.

[hea3ven]

Yeah, I've tried to use the substitution alias feature several times for some time and it has always been broken for different reasons. And yes, when I tried it last the issue was that the new block instance wasn't in the integer id map, if I remember correctly the issue happened when staging from global to the world registry, but I might be wrong.

IMHO as nice as it would be to use this for the actual use cases, this feature might be too powerful and you might see many stupid modders fully replacing blocks when just using events would work. Maybe that's why it hasn't been given much attention, or maybe it's because it's so connected with the registries that any minor change there can break it.

[ShetiPhian]

It worked well in 1.7, just had to register the textures because the method wasn't called.
Block replacement broke in 1.8, haven't touched it in 1.9.

Depending on what your doing it isn't needed anyway.
Replacement block drops the original item.
'PopulateChunkEvent.Pre' for replacing the existing blocks
'PlayerInteractEvent' to make the original item place the new block

[williewillus]

How would PopulateChunkEvent.Pre replace placed blocks inworld?
I thought that's world-gen only
Also pinging @cpw for this

[ShetiPhian]

Jabelar's code example I based mine on has a notice about needing to set the chunk as modified or it will replace all the blocks again every time the chunk is reloaded.

I'm only replacing some instances of a generated block. It works with new chunks but I have never checked if it works in already generated chunks, just figured it was based on the notice.

[LexManos]

All of that hacks shouldnt be nessasary.
the objects should be aliasing just fine in the registry.
Write a small mod to example it and then document steps to reproduce and when I have time i'll go look into it.

[hea3ven]

I had written a test mod some time ago, if someone want's to go ahead and update it for 1.9, be my guest.

[Choonster]

I have a test mod here, originally written for this thread.

You can see the FML log from a test run here.

Issues I've noticed:

• The block can be placed in the world, but it won't persist through a reload or replace existing occurrences of the vanilla block without the manual ID remapping hack (the FMLModIdMappingEvent handler).
• The block has the correct model in the world, but not in the inventory.
• The block shows up twice in the creative tab.
• IForgeRegistry#getValue always returns the vanilla block/item, Item.getItemFromBlock returns the replacement ItemBlock when called with the replacement Block.

The replacement item seems to work without issue.

[cpw]

sigh I'll have another pass at it. Substitution is a complete pain in the ass.

[Actuarius]

@diesieben07 added labels [Bug]

[williewillus]

Any further progress?

edit: my test mod from before: https://github.com/williewillus/remaptest/blob/master/src/main/java/williewillus/remaptest/RemapTest.java

[williewillus]

Any news? Got another person on irc needing this

[iLexiconn]

7d4bf61

[williewillus]

Excellent!

[marvin-roesch]

I'm not really in need for this anymore, but good to hear it's fixed!

[williewillus]

Does this fix remapping as well?
Testing now

[cpw]

If not, add a test in a PR! I'll add it to the suite. I want as much coverage on uses of registry as possible in test cases. Fail needs to do a lot to it, so want broad test coverage before rewriting.

[williewillus]

Okay, from first glance seems like remapping things from a mod->vanilla is still not working. trying other remappings

Edit: wait I derped, testing again

[williewillus]

Yeah, it doesn't work. My old test mod will probably reproduce this still.

https://github.com/williewillus/remaptest/blob/master/src/main/java/williewillus/remaptest/RemapTest.java

[cpw]

Don't want a test mod, they can't be properly automated. See the stuff in src/test and try and build a straight junit test case.

[williewillus]

Actually I don't think my use case is fully supported currently, reading through the code.

I have "botania:prismarine", I want to retain the int ID<->RL mappings for it, but then I want the RL "botania:prismarine" to be aliased directly to "minecraft:prismarine".

e.g. the actual object i get back when I do /give botania:prismarine is the real vanilla prismarine.

The goal is to eventually remove my object and classes, leaving behind only such an alias mapping to migrate players to the vanilla block

The current registry seems to assume that whatever new thing you're subbing in or remapping to is "fresh" and hasn't been registered before.

[williewillus]

maybe it isn't possible and Ishould just remove it lol

[AEnterprise]

@williewillus if it's not possible then it should maybe be made possible, it can see valid usecases for it, mods that backport newer vanilla stuff can migrate cleanly, mods that do major overhauls where alot of things are changed could use this to migrate cleanly. Adventure type mods that are used to make command block minigames would be one example

[cpw]

I think this should work. But you're right, I think it expects the thing being "aliased to" to be new, which is annoying. I'll have to work out a test case. It was on my todo list.