Tubbles / Construction_Drones

Adds ground based construction drones
8 stars 11 forks source link

Why are you overwriting vanilla libraries? #8

Open Mernom opened 2 years ago

Mernom commented 2 years ago

Why is tf_util assigned to the util variable? And not as a local variable on top of that! And the util.empty_sprite function points to a none existent file! Either change the name of your lib, contain it in a local scope, or fix the broken functions.

Tubbles commented 2 years ago

Please note that I am not the original author of this mod, I am just trying to make sure it stays up to date since this mod was abandoned by the creator. With that being said, I suspect what you are referring to is this issue https://mods.factorio.com/mod/Updated_Construction_Drones/discussion/62a550953049a6fb78d74293 ?

I am open for pull requests if you know what needs to be done to solve this.

Rhiftz commented 2 years ago

As far as i can tell(First comment and use of github). You deleted some png files in

More cleaning, formatting

@Tubbles Tubbles committed on Dec 6, 2021

Since i'm by no means a git user, i don't know if this is the issue and neither do i know how to fix it, but the error i get is the one you linked about empty sprite.png

Edit: adding the file from the earlier commit, fixes the error that forces you to disable the mod. I will test further.

Tubbles commented 2 years ago

Thank you @Rhiftz for studying this. I've decided to re-add the image for now, since this seems to be an issue for a lot of people. Although I am not yet able to reproduce this issue I do not know if the fix works. Do you think you could test the pull request #9, or send me a savefile for which I can reproduce this issue so that I can test this myself?

MyNewUsername3000 commented 2 years ago

Hi Tubbles,

having the same issue that Factorio does not boot up without a warning that the file empty-sprite.png was missing and that I should turn off various mods.

I just downloaded the file and the error is gone. The game loads up and everything works fine.

Tubbles commented 2 years ago

Hi Tubbles,

having the same issue that Factorio does not boot up without a warning that the file empty-sprite.png was missing and that I should turn off various mods.

I just downloaded the file and the error is gone. The game loads up and everything works fine.

Thanks, that gives me enough confidence to publish this hotfix. However, the issue is obviously still there, the mod is clearly doing something it shouldn't. In order for me to debug that issue further, I need a savefile that has this issue.

@MyNewUsername3000 could you please send me your savefile or upload it somewhere I can download it?

Tubbles commented 2 years ago

Hotfix released in 1.0.6, could you guys go ahead and update and double check that the issue is fixed for now?

MyNewUsername3000 commented 2 years ago

Hi Tubbles,

Just to help you pinpoint the source of the error: It pops up when starting Factorio after the mods are being loaded, i.e. way before opening a savegame. I am being told that I should deactivate Krastorio2 and aai-industries to proceed or exit Factorio. I can reproduce the error simply by removing emty-sprite.png again.

I don't know the way Factorio or modding works but if I had to guess, I'd say the error is independent from the savefiles as it pops up way before I could even load a saved game. Probably it is just a dependency between the mods. Please find a recent savefile here: https://easyupload.io/qeb6er

Mernom commented 2 years ago

An easy temporary fix: remove the util = tf_util line in data.lua, and add the same line as a local variable in any file which needs util functions.

Tubbles commented 2 years ago

Pushed a hotfix for 1.0.6, a semi-permanent fix will be released with 1.0.7 (in 6928c690)

Tubbles commented 2 years ago

An easy temporary fix: remove the util = tf_util line in data.lua, and add the same line as a local variable in any file which needs util functions.

But aren't we potentially still overriding stuff in tf_util.lua, even though the usage variable is local in other files?

Mernom commented 2 years ago

Hmm, vanilla doesn't set util as a local variable it first defines it... Might need to add a table.deepcopy in the tf util file

Mernom commented 2 years ago

OK, I added it to my fork (with a couple comments unrelated to that particular issue)

theisen1337 commented 2 years ago

OK, I added it to my fork (with a couple comments unrelated to that particular issue)

I tested your code locally, and breaks other Klanon based mods. Seems like this problem might be more wide spread.