This thread is primarily for mod developers to explain their reasoning behind decisions they feel would be (or actually have been) controversial. It is also so players can "see both sides of the story", not just complaints about the aforementioned decisions, and so they can discuss the decisions or even provide new suggestions.
I will start:
========================================================================
For my mods for 1.7, I started adding ItemBlocks (the item-form representation of a block when it is in the inventory or dropped as an item) for the vanilla technical blocks like pumpkin stems and unlit redstone torches. Vanilla does not normally create these ItemBlocks as such blocks are completely unobtainable and thus it assumes there is no need for them. This is because I need to render them in GUIs (for example as a redstone control button's icon), which immediately crashes when there is no corresponding ItemBlock (as it creates a stack of null).
This introduced a problem. Some mods look up items by name, not actual Item reference, and thus this fix resulted in some machines processing the technical blocks (like crop blocks instead of the wheat normally in the recipe).
This was easily solved by renaming the ItemBlocks to have "_technical" in their registration name. However, this introduced another problem:
When trying to load a pre-existing world, FML (ForgeModLoader) triggered the module designed to track when you try to load a save that contains items or blocks that no longer exist in your installation (usually because of the removal of a mod). Behind the scenes, what FML does is blacklist the ID this block was mapped to to avoid one block being transmuted into another (eg TE3 copper ore turning into IC2 mass fabricators if the mass-fab took over the ID that used to represent copper). With ItemBlocks, it also tries to remove the reference to the corresponding block.
This is where the problem lies. For vanilla blocks, the underlying block still exists, causing an "Illegal State" error. The game tries and fails to load the level.dat file and its backup, so FML then regenerates the registry of what IDs correspond to what block. This completely destroys the save, as it scrambles all the modded blocks, and usually results in an immediate and permanent crash.
To get around this problem, I used ASM (runtime class modification, basically a way to change code as the game launches, thus modifying behavior) to tweak one of FML's classes, the GameData class, which manages the item registry. The change makes it so that FML will not attempt to blacklist an ID if the block is a vanilla one, thus solving the problem.
However, this decision has been very strongly criticized, not on the basis of the actual code but in principle of using ASM on FML. The three main criticisms and my responses are below:
Why not just stop creating the ItemBlocks for technical blocks?
OMG WHY ARE YOU ASM-ING INTO FML ARE YOU INSANE!?!?!11!?
Why not just make this a Pull Request into Forge?
Please remain civil, and keep in mind that technical advice from those without technical knowledge is likely to be of little value.
I will start:
========================================================================
For my mods for 1.7, I started adding ItemBlocks (the item-form representation of a block when it is in the inventory or dropped as an item) for the vanilla technical blocks like pumpkin stems and unlit redstone torches. Vanilla does not normally create these ItemBlocks as such blocks are completely unobtainable and thus it assumes there is no need for them. This is because I need to render them in GUIs (for example as a redstone control button's icon), which immediately crashes when there is no corresponding ItemBlock (as it creates a stack of null).
This introduced a problem. Some mods look up items by name, not actual Item reference, and thus this fix resulted in some machines processing the technical blocks (like crop blocks instead of the wheat normally in the recipe).
This was easily solved by renaming the ItemBlocks to have "_technical" in their registration name. However, this introduced another problem:
When trying to load a pre-existing world, FML (ForgeModLoader) triggered the module designed to track when you try to load a save that contains items or blocks that no longer exist in your installation (usually because of the removal of a mod). Behind the scenes, what FML does is blacklist the ID this block was mapped to to avoid one block being transmuted into another (eg TE3 copper ore turning into IC2 mass fabricators if the mass-fab took over the ID that used to represent copper). With ItemBlocks, it also tries to remove the reference to the corresponding block.
This is where the problem lies. For vanilla blocks, the underlying block still exists, causing an "Illegal State" error. The game tries and fails to load the level.dat file and its backup, so FML then regenerates the registry of what IDs correspond to what block. This completely destroys the save, as it scrambles all the modded blocks, and usually results in an immediate and permanent crash.
To get around this problem, I used ASM (runtime class modification, basically a way to change code as the game launches, thus modifying behavior) to tweak one of FML's classes, the GameData class, which manages the item registry. The change makes it so that FML will not attempt to blacklist an ID if the block is a vanilla one, thus solving the problem.
However, this decision has been very strongly criticized, not on the basis of the actual code but in principle of using ASM on FML. The three main criticisms and my responses are below:
Why not just stop creating the ItemBlocks for technical blocks?
The ItemBlocks are required for rendering the blocks (all of which are unobtainable technical blocks like pumpkin stems, unlit redstone torches, and portal blocks) as items in the inventory. Otherwise, the ItemStack created will have a null item and will immediately crash. Special-casing 40 different renderers - all of which would have to be hand-written for these blocks - is an extremely onerous task.
Additionally, removing them does not remove the need for this ASM code until it is guaranteed that everyone has loaded the world without the itemblocks and with this code present, as only then is the world safe to load without this code or the itemblocks. Given the difficulty in getting many players to update at all, let alone use specific versions in sequence, such an approach is completely nonviable. Destroying existing worlds is absolutely unacceptable.
Additionally, removing them does not remove the need for this ASM code until it is guaranteed that everyone has loaded the world without the itemblocks and with this code present, as only then is the world safe to load without this code or the itemblocks. Given the difficulty in getting many players to update at all, let alone use specific versions in sequence, such an approach is completely nonviable. Destroying existing worlds is absolutely unacceptable.
OMG WHY ARE YOU ASM-ING INTO FML ARE YOU INSANE!?!?!11!?
While this approach certainly looks insane, its only effect is to compensate for an unexpected edge case and oversight in FML. It has no other effect on the code, and barring JVM errors, cannot cause any other issues.
Why not just make this a Pull Request into Forge?
Because this code is to fix a rare edge-case, and said edge-case is one that design purists feel should never have happened to begin with, all mentions of putting this natively into Forge/FML were met with derision and hostility. While I never actually made a PR, when I mentioned my initial intentions to others, I was either laughed at or flippantly told "maybe you shouldn't be rendering these blocks".
Additionally, at the time this code was written, all development on Forge/FML for MC 1.7 had been frozen. As such, even if the fix had been included, it would have only made it into 1.8, making it far too late to be of any use.
I now open the thread for other developers to explain their reasoning in a similar fashion, or for the community to discuss.Additionally, at the time this code was written, all development on Forge/FML for MC 1.7 had been frozen. As such, even if the fix had been included, it would have only made it into 1.8, making it far too late to be of any use.
Please remain civil, and keep in mind that technical advice from those without technical knowledge is likely to be of little value.
Last edited:
