Skip to content
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

Search every overmap for npcs to continue as once you've died #57762

Merged
merged 2 commits into from
Jun 2, 2022

Conversation

perryprog
Copy link
Contributor

Summary

None

Purpose of change

This amends commit 8f900e5, as it turns out that was an accidental revert of PR #56220, which was a bugfix.

Describe the solution

To correctly hide the "Continue as one of your followers" message, every overmap and each npc of every overmap should be iterated through, which is what this commit does by way of adding an EOC condition based off of the already existing npc_allies one.

Describe alternatives you've considered

To use game::get_follower_list, which is what avatar::control_npc_menu uses. While I originally didn't realize that method existed, I think it's better not to use it in this one case: I noticed a few instances during testing where I was getting the option to take control of my dead self. (Specifically: as a new player I would get a follower, die, control the follower, die, and get the option to control the original player where if I did I would immediately be put into the "Watch the last movements of your life" dialog. For whatever reason, I could never get that to occur using this PR's current patch.

(Note: I suspect I'm missing something here because in one of the tests I did I was controlling an NPC with 1 NPC follower and the player having just died. In that case, I should've seen the take-control menu as the count my code gave would've been 1... this didn't happen, however)

Testing

I tested just about every circumstance I could think of (multiple followers, being in the overmap and out of the overmap of the player's followers, dying with no followers) and I didn't notice any unexpected behavior. I still suspect there could still be something up as described above, though since there's no current buggy behavior I can get to happen, I don't think it's too much of an issue.

Additional context

@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON [Markdown] Markdown issues and PRs EOC: Effects On Condition Anything concerning Effects On Condition labels May 16, 2022
@perryprog perryprog force-pushed the long-distance-friends branch from 60c546c to 0b49646 Compare May 16, 2022 22:25
This amends commit 8f900e5, as it turns out that was an accidental
revert of PR CleverRaven#56220, which was a bugfix.

To correctly hide the "Continue as one of your followers" message, every
overmap and each npc of every overmap should be iterated through, which
is what this commit does.
@perryprog perryprog force-pushed the long-distance-friends branch from 0b49646 to 296f105 Compare May 16, 2022 22:25
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels May 16, 2022
@dseguin dseguin added the NPC / Factions NPCs, AI, Speech, Factions, Ownership label May 16, 2022
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label May 17, 2022
@PatrikLundell
Copy link
Contributor

Have you tested what happens if your only follower is assigned to a base camp or is currently on a mission (which could be a camp one or one for the Tacoma commune), as well as busy doing some ordinary task (clean up, chop trunks into planks, dismantle vehicles etc.)?
That's not really related to the fix, but to the underlying functionality.

@perryprog
Copy link
Contributor Author

Have you tested what happens if your only follower is assigned to a base camp or is currently on a mission (which could be a camp one or one for the Tacoma commune), as well as busy doing some ordinary task (clean up, chop trunks into planks, dismantle vehicles etc.)?
That's not really related to the fix, but to the underlying functionality.

I just checked—very surprisingly, it works fine for an NPC assigned to a basecamp mission. They sort of just re-appear where they disappeared from if you choose to possess one. I haven't tested out any other combinations, though.

@kevingranade kevingranade merged commit 6718bdf into CleverRaven:master Jun 2, 2022
bombasticSlacks pushed a commit to bombasticSlacks/Cataclysm-DDA that referenced this pull request Jun 10, 2022
…Raven#57762)

* Search every overmap for npcs to continue as

This amends commit 8f900e5, as it turns out that was an accidental
revert of PR CleverRaven#56220, which was a bugfix.

To correctly hide the "Continue as one of your followers" message, every
overmap and each npc of every overmap should be iterated through, which
is what this commit does.

* fixup! Search every overmap for npcs to continue as
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. EOC: Effects On Condition Anything concerning Effects On Condition [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants