-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Use clear_npcs in clear_map #30051
Merged
kevingranade
merged 2 commits into
CleverRaven:master
from
jbytheway:clear_npcs_in_map_helper
Apr 30, 2019
Merged
Use clear_npcs in clear_map #30051
kevingranade
merged 2 commits into
CleverRaven:master
from
jbytheway:clear_npcs_in_map_helper
Apr 30, 2019
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clear_map had a similar flaw to the one fixed in CleverRaven#30031 affecting npc_can_target_player. Fix it in the same way. This wasn't obviously affecting any tests, but it makes sense to fix it pre-emptively.
ifreund
added
<Bugfix>
This is a fix for a bug (or closes open issue)
Code: Tests
Measurement, self-control, statistics, balancing.
NPC / Factions
NPCs, AI, Speech, Factions, Ownership
labels
Apr 29, 2019
mlangsdorf
approved these changes
Apr 29, 2019
jbytheway
changed the title
Use clear_npcs in clear_map
[WIP] Use clear_npcs in clear_map
Apr 29, 2019
Marking this WIP because the Travis error looks problematic. |
Some tests (default_overmap_generation_has_non_mandatory_specials_at_origin) overwrite overmaps, which leads to orphaned NPCs. Unloading and reloading the NPCs ensures that all the active NPCs are known to the overmap.
jbytheway
changed the title
[WIP] Use clear_npcs in clear_map
Use clear_npcs in clear_map
Apr 29, 2019
Hopefully now better. |
jbytheway
added a commit
to jbytheway/Cataclysm-DDA
that referenced
this pull request
Apr 30, 2019
After CleverRaven#30051 clear_creatures no longer clears NPCs. Add a call to do that in this test where it is needed.
jbytheway
added a commit
to jbytheway/Cataclysm-DDA
that referenced
this pull request
Apr 30, 2019
After CleverRaven#30051 clear_creatures no longer clears NPCs. Add a call to do that in this test where it is needed.
jbytheway
added a commit
to jbytheway/Cataclysm-DDA
that referenced
this pull request
Apr 30, 2019
After CleverRaven#30051 clear_creatures no longer clears NPCs. Add a call to do that in these tests where it is needed.
jbytheway
added a commit
to jbytheway/Cataclysm-DDA
that referenced
this pull request
Apr 30, 2019
After CleverRaven#30051 clear_creatures no longer clears NPCs. Add a call to do that in these tests where it is needed.
jbytheway
added a commit
to jbytheway/Cataclysm-DDA
that referenced
this pull request
May 1, 2019
After CleverRaven#30051 clear_creatures no longer clears NPCs. Add a call to do that in these tests where it is needed. Also, in clear_map, remove NPCs before vehicles to avoid unboarding errors (as already hinted at in npc_test.cpp). Simplify npc_test to use the new improved clear_map. Ultimately, this is all a workaround for the fact that vehicle detaching does not properly mark NPCs as not in the vehicle. That should also be fixed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
<Bugfix>
This is a fix for a bug (or closes open issue)
Code: Tests
Measurement, self-control, statistics, balancing.
NPC / Factions
NPCs, AI, Speech, Factions, Ownership
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
SUMMARY: None
Purpose of change
clear_map
had a similar flaw to the one fixed in #30031 affectingnpc_can_target_player
.Describe the solution
Fix it in the same way, using
clear_npcs
added in #30031. Having done so, stop callingunload_npcs
inclear_creatures
.Additional context
This wasn't obviously affecting any tests, but it makes sense to fix it pre-emptively while I'm thinking about this subject.