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

Replace alot of raw strings in faction_camp.cpp and make use of static const where appropriate #72809

Closed

Conversation

Procyonae
Copy link
Contributor

@Procyonae Procyonae commented Apr 3, 2024

Summary

Bugfixes "Faction camp clearcutting works on rural roads again"

Purpose of change

Fixes #72808

Describe the solution

Removes references to migrated omt ids
Swaps out the gross if else for a generalised map that works for all rotation types for easy future additions without needing extra logic

Could do with #72558 merging first so I can reuse it to set the parameters for the road type to be the same pre/post cut

Describe alternatives you've considered

Using a set for the forest oters to convert to field without rotation handling and dealing with rural road and it's linear rotation as a special case which would be shorter and more efficient than the map

I want to replace several things using std::string s in place of ids with ids but I'll do that after this has merged rather than delaying the bugfix

Unhardcoding the std::unordered_map but that would require the above so log_sources uses the same check

Testing

None yet, faction camp testing is a PITA

Additional context

@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 Apr 3, 2024
@PatrikLundell
Copy link
Contributor

Yes, finding places to create a camp that allows you to order logging on various types of terrain is tricky.

@Procyonae
Copy link
Contributor Author

Yes, finding places to create a camp that allows you to order logging on various types of terrain is tricky.

How do you actually test camp stuff in a timely manner? Do you just have several saves setup in different situations and reuse them or?

@PatrikLundell
Copy link
Contributor

No. I generally start with my latest save, teleport to a suitable location, debug spawn and recruit a companion and construct the simplest type of camp I can (frequently bare bones), and then order the new companion do do what needs to be done. I tend to actually copy the save and then save as the basics have been set up so I can repeat what needs to be done from a reasonable starting point.
For logging tests this means looking for a location with access to "forest type X" tiles of the types to be tested within the logging distance, and I typically am able to deal with a few right off the camp in my latest save (and so need no new camp and save for those test cases).
I did fail to find a few types when implementing the expanded logging, though, despite looking far and wide. I suspect hacking some overmap tiles that haven't been generated yet might also work, but I haven't tried that approach.

When it comes to testing camp construction testing it's a matter of building a starting camp and then progress through the construction one material at a time, reverting to the starting camp save after each material. It's rather tedious, and it will take a number of days, dealing with a few materials per day (its numbingly boring for one, and the risk of slipping up increases with the boredom).
When going through every blasted base camp it takes a lot of time. I think the construction material support is sufficiently robust now that you can test each construction step with a single material and then shift between materials for each step. Still boring and a lot of work.

@Procyonae
Copy link
Contributor Author

Procyonae commented Apr 8, 2024

If you wanted to open an issue describing new debug features that would help you I'd take a crack at adding them, I imagine the tediousness of testing camp stuff puts off other peeps contributing to it, I know I personally haven't touched camp maps for this reason.

@PatrikLundell
Copy link
Contributor

I don't really see how you can speed up base camp testing by much. The most annoying elements is incessant portal storm spam (due to the debug waits) and assassins retroactively sent for having, in the past, performed the the old guard quest for a now obsolesced reward that keeps spamming the save loading to taunt you.

There's no real way around faction camp testing tedium since you somehow need to go through all the different paths in the construction tree to verify they are generated as intended, and getting to a particular construction you've modified still requires you to get the previous ones. Sure, you can save some work by copying a basic save and then save at appropriate points (such as after ironing out all the issues with the first construction, before going on to the next one, and similarly when you get to a branch you can save at the last common denominator construction).
I'd still recommend the using of every palette's defined tile at least once in order to catch typos, but that's usually easily done with the materials palettes because they tend to have only a small number of tiles which are all used in most structural constructions, and there usually aren't too many other variant tiles (beds and cooking facilities, typically, with beds typically present in many constructions and thus possible to vary as part of the standard branch coverage, but stoves and the like tend to require repetition).

I guess zero (or, rather, single tick) debug construction times and automatic debug materials fulfillment might help. Apart from that, you still need to find whatever base location you're going to test or whatever environment features you want to affect (like logging), as I don't think you can expect debug changed OMTs to behave as naturally spawned ones with 100% fidelity.
Finding locations would be easier if the search function searched the whole revealed map rather than being limited to a single overmap (resulting in you finding 0 matches even though you know you've seen a matching site somewhere when scrolling around, but not being able to find the bugger again).

@Night-Pryanik
Copy link

Closing as stale. If you wish to continue working on this, ping me to reopen.

@Procyonae Procyonae changed the title Faction camp clearcutting works on rural roads again Replace alot of raw strings in faction_camp.cpp and make use of static const where appropriate Dec 10, 2024
@Procyonae
Copy link
Contributor Author

@Night-Pryanik I take it you can't reopen this either bc I rebased it?

@Night-Pryanik
Copy link

Yes, probably. The reopen button is simply inactive.

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.

Faction camp clearcutting borked
3 participants