[1.1.88] Fail to load past 9%

[TheSAguy]

Hi,

I have a bit of a weird bug report here. I recently updated my mod, adding in a some new graphics.
When loading the game with the new graphics, Natural_Evolution_Enemies_1.1.21.zip, the game does not get past 9% loading.




Works fine with the version before the graphics change, Natural_Evolution_Enemies_1.1.18.zip.

I've attached a save.
Mods: https://mods.factorio.com/mod/Natural_E ... /downloads

The graphics I added was from Zerg
All I did was run PNG Gauntlet on the files to make them a little smaller.

Initial bug report. Somehow Alien Biomes seems to be involved, but not sure how....

I'm playing with 188 mods in my personal game, Including the new NE Graphics and Alien Biomes without any issues. But was now reported by two other users having this issue.

Any assistance in getting this resolved would be appreciated.
Thanks,

[Rseding91]

The error seems to be in alien biomes with it's collision_mask_util_extended logic. It is stuck trying to get a collision mask in some infinite loop.

[boskid]

Thanks for the report, however this is a mod error.

In this specific case, there are 2 mods: alien-biomes and Natural_Evolution_Enemies both carrying a separate copy of the `collision-mask-util-extended.lua`, respectively in `__alien-biomes__/collision-mask-util-extended/data/collision-mask-util-extended.lua` and `__Natural_Evolution_Enemies__/lib/collision-mask-util-extended.lua`.

Issue with the collision-mask-util-extended.lua is hidden literally in the second line of the code:

Code: Select all

local collision_mask_util_extended = require("__core__/lualib/collision-mask-util")

This line does not return new, independent instance of collision-mask-util from the core but it provides a reference to existing instance, which means all the functions added by this util are added to the shared global instance of the `collision-mask-util` that was already loaded once.

As for the freeze how it happened, when Natural_Evolution_Enemies mod loads its own instance of `collision-mask-util-extended`, it tries to install its own hook into `get_default_mask` function by first copying original handler to the `collision_mask_util_extended.get_default_mask_vanilla` variable (this is now pointing at the core's original implementation of get_default_mask) and then installing a new handler in place of the `get_default_mask`, overwriting the shared instance of this library. At this point in time we have following:
- get_default_mask_vanilla points at original core's handler
- get_default_mask points at a handler from Natural_Evolution_Enemies's copy of collision-mask-util-extended which is calling the get_default_mask_vanilla in case the request is not for a "unit"

After this, the alien-biomes mod loads its data-final-fixes stage, which loads second copy of this library, also working on the shared global state. It reaches the line where it copies the `get_default_mask` function (supposed to be from core but it is already overwritten by Natural_Evolution_Enemies copy of this library) into the get_default_mask_vanilla, but that variable was already set, so it overwrites this function. Then it installs a separate handler over the `get_default_mask` function, so now we have following:
- get_default_mask_vanilla points at a handler from Natural_Evolution_Enemies's which will call get_default_mask_vanilla in case the request is not for a "unit". This means it will enter an infinite loop as it will start calling itself
- get_default_mask points at a handler from alien_biomes's copy of collision-mask-util-extended which is calling the get_default_mask_vanilla in case the request is not for a "unit"

Then the alien-biomes calls `get_mask` for a car, it goes to the hook from alien_biomes copy of this library, it sees its not a unit so it tries to call `get_default_mask_vanilla`, this is a hook from Natural_Evolution_Enemies copy of this library, it also sees its not a unit, it tries to call `get_default_mask_vanilla` but that already points at itself causing a complete freeze.

This issue would be detected earlier if this library would contain an assert when overwriting `get_default_mask_vanilla` that it should be `nil` before as in all other cases it is just a programming error.
This issue could be fixed by having `collision-mask-util-extended` perform a deepcopy of the collision-mask-util obtained from the `require` call
This issue could be fixed if this library would not persist the original `get_default_mask` function inside of the `collision_mask_util_extended` because it could be mutated later, but it would store the original handler in a local variable that is still accessible from the new handler being installed, that handler would have an access to a local that would be part of a closure and as such does not need to be persisted inside of the `collision_mask_util_extended`

[TheSAguy]

Thanks for looking into this, really appreciate it.

Sorry for posting it as a bug.