cataclysmbnteam / Cataclysm-BN

Cataclysm: Bright Nights, A fork/variant of Cataclysm:DDA by CleverRaven.
https://docs.cataclysmbn.org
Other
698 stars 272 forks source link

feat(port): Implement `GLOBALLY_UNIQUE` #5723

Open Zlorthishen opened 6 days ago

Zlorthishen commented 6 days ago

Checklist

Required

Optional

-->

Purpose of change

ports dda change

autofix-ci[bot] commented 6 days ago

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](https://docs.cataclysmbn.org/en/contribute/contributing/#code-style), 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 commented 6 days ago

i get evac center needing to be unique n because of unique npcs it spawns, but may i ask why military bases and aircraft carrier need be globally unique?

Generally speaking I'm ok with adding more specific military locations and tying them into quests than just having a single generic military base.

If we want to go with players being able to access military loot sustainably we can add national guard armories and an airbase, plus garrisons.

The carrier is a bit of a weird one as finding several carriers in lakes would be more of a stretch than finding one.

Zlorthishen commented 6 days ago

i get evac center needing to be unique because of unique npcs it spawns, but may i ask why military bases and aircraft carrier need be globally unique?

those were just from the original dda pr, but there are probably a few other locations that could be Globally Unique

edit: I removed the flag from the military base, because it isn't unique enough to be a one-off location

chaosvolt commented 5 days ago

Testing in build 2024-11-14:

  1. Success
  2. Success
  3. Success
  4. Success
  5. Success
  6. Success
  7. Success
  8. Success
  9. Success
  10. Success

Testing in compiled PR:

  1. Success
  2. Failure
  3. CRASHES
  4. Success
  5. Success
  6. Success
  7. Success
  8. Success
  9. Success
  10. Success

Both tests done with default world settings. Only having it fail to generate once is arguably not fatal to this PR since the sample size is low enough and the carrier scenario is suspected to do this occasionally even without this PR, but the crash is definitely concerning. image

chaosvolt commented 5 days ago

I've yet to get it to crash again to try and grab more info, but I did just get four failures to generate in a row.