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

Tests: clear_map() clears basecamps #78016

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

ShnitzelX2
Copy link
Contributor

@ShnitzelX2 ShnitzelX2 commented Nov 20, 2024

Summary

Bugfixes "clear basecamps for tests in clear_map()"

Purpose of change

When the test npc_talk_location was run after any set of tests that makes a basecamp (e.g. camp_calorie_counting), it would fail. This was because camps could still exist on the overmap tile from those previous tests, which would count as a dialogue response for this:

{
  "text": "This is a faction camp any test response.",
  "topic": "TALK_DONE",
  "condition": { "u_at_om_location": "FACTION_CAMP_ANY" }
},

and the dialogue counts would be larger than expected.

Describe the solution

EDIT: clear_map() removes all basecamps, added it to the test. Added a REQUIRE that makes sure there aren't any camps found.

Describe alternatives you've considered

Testing

Ran npc_talk_location by itself and with camp_calorie_counting before it, no errors.

EDIT: Ran all tests locally, no issues, check for test errors in CI

Additional context

The faction camp OM used (faction_base_camp_11) is legacy, should probably be removed

@github-actions github-actions bot added NPC / Factions NPCs, AI, Speech, Factions, Ownership Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Nov 20, 2024
@mqrause
Copy link
Contributor

mqrause commented Nov 20, 2024

I think you should put clearing camps into clear_map.

@RenechCDDA
Copy link
Member

Wow, I never thought about that. Agree with mqrause that camps should be cleared with clear_map().

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Nov 20, 2024
@ShnitzelX2
Copy link
Contributor Author

I think you should put clearing camps into clear_map.

That's a good idea. It looks like removing the camps in just this test created another error, so I might as well give it a shot. May submit to a new PR if it ends up working.

@ShnitzelX2 ShnitzelX2 changed the title npc_talk_test: npc_talk_location checks for preexisting basecamps Tests: clear_map() clears basecamps Nov 20, 2024
@github-actions github-actions bot added BasicBuildPassed This PR builds correctly, label assigned by github actions and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels Nov 20, 2024
@RenechCDDA
Copy link
Member

Not trying to bikeshed, but you might want to also erase it from the camps list stored on the player character

std::set<tripoint_abs_omt> &known_camps = get_player_character().camps;
known_camps.erase( omt_pos );

overmapbuffer::remove_camp does not do this, normally it's handled inside basecamp::abandon_camp()

fix test npc_talk_location
@github-actions github-actions bot added the Player Faction Base / Camp All about the player faction base/camp/site label Nov 25, 2024
@Night-Pryanik Night-Pryanik merged commit 9cb8112 into CleverRaven:master Nov 25, 2024
18 of 25 checks passed
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 <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. json-styled JSON lint passed, label assigned by github actions NPC / Factions NPCs, AI, Speech, Factions, Ownership Player Faction Base / Camp All about the player faction base/camp/site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants