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

feat(port): increase default search range for mission placement, sanity-check search properties for scenario missions #5900

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

chaosvolt
Copy link
Member

Checklist

Required

Optional

Purpose of change

This extends the range of mission search if search range is undefined, along with some other improvements ported from DDA. Shifting missions to not be limited in search radius will be useful for when #5723 is merged, as it may become a drunken crapshoot how far off a true-unique location will end up being.

Describe the solution

C++ changes:

  1. In mission.h, set the default search range to the expanded value used in the above DDA PR. I don't think it's actually unlimited but it is de-facto vastly more than a playthrough will be able to exceed.
  2. In mission_util.cpp, ported over the message change where failure to find a mission target now tells you the distance it was told to search.

JSON changes:

  1. Removed no longer nessecary search ranges from all missions where location is non-random, since the default search range will be much higher than the values being used.
  2. The Last Delivery scenario given some hangup protection by being set to a max distance of 200 tiles, also defining reveal radius and min distance.
  3. Misc: Expanded search radius of One Last Assassination to 200 tiles, set to be non-random.

Describe alternatives you've considered

Doing it closer to how DDA's implementation does it would have it also print a load error for even bother to use search radius for non-random mission targets. While the feature is not strictly needed now that the default search range is huge, there is the possibility that for performance one might want to limit search range for missions where the mission designer can be reasonably certain the target location will be found within that range.

Testing

  1. Checked affected JSON files for syntax and lint errors.
  2. Compiled and load-tested in a zero city size world.
  3. Spawned in a refugee center, which I know in this world setting will be the only one to exist.
  4. Warped a few overmap chunks away from the center and spawned in an occupied lumbermill.
  5. Advanced through missions, they were able to find the refugee center despite it being 361 map tiles away.
  6. Checked affected C++ files for astyle.

image

One thing I noticed during testing: If a scenario is set to start a mission, it can hang up and take forever to search if it's using a big enough search distance:
image

One Last Assassination and The Last Delivery should be more likely just fail with a quick skippable error message and break the mission instead of being stuck searching forever with these mission settings when used on particular finicky city size settings, which is still not ideal but better than it being stuck loading forever. Checked in my old desktop build (still on build 2024-11-26) to confirm that the mission can already fail on oddball city size/distance settings in existing builds.

Additional context

PR being ported, "Remove mission search radius from most missions" by @RenechCDDA: CleverRaven/Cataclysm-DDA#78949

Co-Authored-By: RenechCDDA <84619419+RenechCDDA@users.noreply.github.com>
Co-Authored-By: Maleclypse <54345792+Maleclypse@users.noreply.github.com>
@github-actions github-actions bot added src changes related to source code. JSON related to game datas in JSON format. labels Jan 7, 2025
Copy link
Contributor

autofix-ci bot commented Jan 7, 2025

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

@RoyalFox2140 RoyalFox2140 merged commit 8bcd3b5 into cataclysmbnteam:main Jan 8, 2025
16 checks passed
@chaosvolt chaosvolt deleted the mission-search-stuff branch January 8, 2025 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format. src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants