eott / preciouspreciouscaffeine

[Done] A mod for the game Factorio, adding coffee products and technology to the game
2 stars 3 forks source link

Crash when died while under caffeine effects #6

Open ADDSynth opened 1 week ago

ADDSynth commented 1 week ago

Screenshot 2024-10-04 125729
This crash likely happened when I died while under the effects of caffeine. Then I respawned, and the coffee counter kept going down, then the crash happened when it reached 0.

Possibly crashed because it was trying to take the effects off when I had no caffeine effects.

I am playing with quite a few mods, so let me know if you need the list. Am playing with the Death Curses mod, which also gives effects to player after death.

eott commented 1 week ago

Status: Could not replicate, but likely candidate for issue identified.

Attempt to reproduce nr 1:

  1. Started Factorio with only PPC as mod activated
  2. Loaded a vanilla save file
  3. Consumed caffeine to get a caffeine level of 100%
  4. Died (to nuke, but shouldn't matter)
  5. Chose Respawn and respawned
  6. Waited for the counter to reach zero and confirmed that running speed is still faster while the counter is above zero
  7. When the counter reached zero, the game didn't crash and the running speed buff was removed

Attempt to reproduce nr 2:

  1. Started Factorio with PPC and Death Curses as mods activated
  2. Loaded a save file that previously only had PPC activated
  3. Repeated Steps 3 through 6 as above
  4. When the counter reached zero, the game didn't crash and the running speed buff was removed

Hypothesis This is the code causing a crash:

player.character_crafting_speed_modifier = (
    player.character_crafting_speed_modifier
    - buffed_crafting_speed_modifier
)

Given that the Death Curses mod likely also contains code modifying the crafting speed modifier, specifically reducing it as a curse, this will clash with PPC doing the same. Probably a curse was applied that reduces the modifier and when the caffeine ran out, PPC attempted to reduce it even further, below the threshold of -1.

The problem is the mods have no way of communicating with each other which modifiers modifications are currently active. A possible fix for the crash would be to never reduce the modifier below -1 in PPC, however this could potentially have a followup issue in that the reduction of the modifier after the counter reached zero is not always the same in magnitude as the initial buff, leading to an accumulation of the modifier if the process is repeated.

Next steps: I will have a look at how Death Curses handles this mechanic and if a solution becomes apparent. Unfortunately it might be the case that using both mods at the same time is not safe in regards to this issue and should be avoided.

ADDSynth commented 6 days ago

Wow. I am impressed by how professional and detailed your analysis is. Though I am seeing github issue responses look like that more and more. I wonder if that's a developer thing or github thing, that everyone just decided to do or that's how you're supposed to do it.

I know your the developer of this mod and have much more experience in modding Factorio than me. But I still feel like offering my input and suggestions. It's my own way of trying to be helpful when I can't solve the problem directly. But I also hope I don't come off as annoying.

Possible Solution: (my first thought) Step 1: Check if Death Curses is active, by asking Factorio api if a mod ID is loaded. Step 2: Use Death Curses api (if it has one) to check if a death curse is active, if a player is undergoing the slow crafting curse or slow moving curse, and/or get the actual constant value that Death Curses lowers the player's values by (again if it has the value saved as a constant and not a magic number.) Step 3: To take the caffeine effects off, then you set the player's craft speed / movement speed to normal - the Death Curse number. - But as you said there's no way to access any of Death Curse's code / api, so this isn't likely to happen. - And now I realize you'll have to update to also check for other mods that also change the crafting speed in the future. This might not be the best solution.

Real Solution: (a lot better solution) The Death Curses mod should be the one to check if the Precious, Precious Caffeine mod is installed, and just choose to SKIP selecting the slow crafting curse and slow movement speed curse. The Death Curses mod should add Mod Options to let players choose which Death Curses can be chosen to be honest.

Nuclear Solution: When the player clicks on the Coffee button to start the caffeine effects, cache the player's crafting speed at that moment (before you change it). Then for every tick the caffeine effects are active, check the player's crafting speed to see if it changes, and update your cached value you saved before. Then when coffee effects are over, revert the player's crafting speed back to your saved value.

This will catch ANY mod that changes the crafting speed while caffeine effects are active.

Updating your saved value should be the first thing you do in your tick function, because other mod's tick functions may have processed before your function, and already changed the player's values. (I've actually encountered this problem in one of my projects.)

I was going to suggest the Real Solution as the one I think you should do, but now I think you should do the Nuclear Solution, since for the cost of one if check in the tick function, it is the safest option and handles other mods messing with the crafting speed as well. And you don't have to wait for others to change their code, you can change your code now. And Death Curses doesn't have to skip selecting the slow crafting curse or slow moving curse. Both the curse and caffeine effects can work at the same time.

eott commented 4 days ago

Update

I managed to replicate the issue (it's really just reloading until the right curse lands) and implemented a fix in version 2.0.1, which was released just now.

The bugfix is a specific compatibility check with Death Curses and likely won't work with other mods modifying the crafting/running speed modifier. The solution I would've prefered is setting up a unified API for modifying the modifiers such that multiple effects from different mods can coexist and work correctly without any compatibility check. This would probably become part of one of the library mods, then add a dependancy and use the library in PPC and convince other mod authors to do the same. However with Space Age just around the corner, I'll save the effort for when/if I update PPC to 2.0 and/or SA.