-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't lose your heads over infinite loops #35808
Conversation
Don't forget to update the comment about "cranial detonation". Similarly, I don't think the new comment "instead of destroying NPC" makes sense because it refers to an aspect of the code that will no longer exist once this PR is merged. |
Co-Authored-By: eso <[email protected]>
Co-Authored-By: eso <[email protected]>
I think having a mention that this was the historical practice isn't without merit there, but I'm personally not particularly concerned either way. |
…wn/Cataclysm-DDA into npc_faint_not_explode
As an additional thought: is there merit in giving the player a pop-up before talking to an NPC that was suspended, to let them know/remind them that they're trying to wake up an NPC that was suspended due to an infinite loop? |
You could certainly say something like "A previous version of the game just destroyed the NPC in this case", but the current wording suggests that destroying the NPC is somehow a possibility. You can direct the reader to check previous commits if they're interested, but you can't rely on them already knowing how the code used to be. |
"Hi, I'm in a debug coma, want to try and wake me up to see if the problem was fixed?" or do we try and keep some form of immersion ? and pretend its like fainting debug goat disease, idk. |
I don't think immersion is necessary here, given that an error is a prerequisite. my slightly frazzled brain wants to suggest something like:
but I know that's wordy. at the least, I think this isn't a place where we need to try to be cutesy, and should instead be clear about what's going on. |
'Your npc has finished rebooting, do you want to reactivate them?' <that would be amusing |
Summary
SUMMARY: Bugfixes "NPCs faint when infinite looping, instead of dying."
Purpose of change
Despite it being a constant and slightly humorous part of developing NPCs and adding new bugs to them, it really isnt needed that NPCs heads explode when they are infinite looping.
This is a leftover from the very early days of the game when NPCs werent as important.
Describe the solution
Ive added a reboot() function to NPCs that resets a lot of their AI state, resets their activities, goal points, path points, destinations , everything, then wacks them over the head with a metaphysical hammer, so they faint into a coma.
This is preferable to killing them.
Now they are asleep, they dont process anymore, and hopefully the reboot has fixed their brain, so you can talk to them and tell them to wake up, hopefully they are fixed, if not, theyll immediately faint again, and you can keep them in a coma while you wait for a bugfix, instead of losing your beloved NPC forever in a cranial conflagration.
This should cover most causes of infinite loops, with the list of things that are reset/cleared
Describe alternatives you've considered
N/A
Testing
Deliberately induced an infinite loop with an activity that didnt subtract moves at all.
NPC fainted, got debugmsg as usual, told them to wake up, they were fine again, as they no longer had an activity.
Additional context