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

Remove bed requirement for base expansion #67908

Merged

Conversation

PatrikLundell
Copy link
Contributor

@PatrikLundell PatrikLundell commented Aug 26, 2023

Summary

None

Purpose of change

Fix #67889
Fix #67848

Describe the solution

Remove the usage of "bed" tokens as a condition for base camp expansion, and added removal of the order when all potential expansion slots have been used (not to be confused by available slots: there are no checks to weed out slots the base camp can't expand into).

As a bonus, the probably bug behind #67848 was fixed as that was required to test the target change.

Describe alternatives you've considered

N/A

Testing

  • Loaded an old save used for base camp testing and created a field base camp version 1 (one type that used to depend on bed counts).
  • Send companion to survey an expansion and tried to create the first one in the list in the NW location, causing the game to terminate.
  • Debugged the cause of the game blowing up and found it to be initiation of a list to be one too few. Odd, since I believe exactly the same bug was encountered and fixed when the functionality to hide orders was introduced. Remembered that someone had issued a bug report a few days ago, giving rise to the suspicion that someone has (re)introduced the bug recently, but I haven't tried to investigate this.
  • Fixed the bug (less than check to less than or equal in a loop).
  • Restarted testing from scratch and created one expansion in each of the 8 expansion directions (different types in different slots, from the list in order, because why not add variation in case there is something there?).
  • Found that the expansion order remained when all slots were filled.
  • Checked the original code and failed to see there was any such check there.
  • Added a check to remove the command when it could no longer possibly be useful.
  • Retested as above (same expansions: the creativity has limits).
  • Verified that the expansion command was no longer available when all 8 expansions had been introduced.

Additional context

Nothing has been done to individual base camp versions to deal with potential references to bed restrictions.

While doing the changes I also searched the code for all usages of "bed", and the only one found was something associated with vehicles. There were also a couple of checks for "BED", so it's probably safe to remove all allocations of "bed" from base camp recipes, as they should have no usages anymore.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Player Faction Base / Camp All about the player faction base/camp/site <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Aug 26, 2023
@PatrikLundell
Copy link
Contributor Author

"Some checks were not successful
3 skipped, 19 successful, and 5 cancelled checks"

What does that mess mean? Mandatory checks seem to be cancelled for no stated reason. Could it be that the logic decided to skip the mandatory JSON test (presumably because no JSON was modified), and then decided that because that test wasn't performed, testing failed, so other tests should be cancelled?

@Maleclypse
Copy link
Member

"Some checks were not successful 3 skipped, 19 successful, and 5 cancelled checks"

What does that mess mean? Mandatory checks seem to be cancelled for no stated reason. Could it be that the logic decided to skip the mandatory JSON test (presumably because no JSON was modified), and then decided that because that test wasn't performed, testing failed, so other tests should be cancelled?

Github hiccups. Sometimes they are github having an issue and sometimes it is the CleverRaven repo running out of free bandwidth to run tests with.

@Maleclypse Maleclypse merged commit 3783fae into CleverRaven:master Aug 30, 2023
@PatrikLundell PatrikLundell deleted the unchained_camp_expansion branch August 30, 2023 06:46
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` json-styled JSON lint passed, label assigned by github actions 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.

NPC camps don't account for beds Segmentation fault upon expanding basecamp to include mechanic shop
2 participants