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

Added multiple camp construction workers #55816

Merged

Conversation

PatrikLundell
Copy link
Contributor

Summary

Features "Added multiple workers to camp construction"

Purpose of change

Allow players to assign multiple workers to camp construction missions.
Fix #53656

Describe the solution

When a camp construction job is started, ask for additional companions after the first one until out of companions or the player escapes out of the selection list. The time taken is divided by the number of companions assigned and only then converted to "work days".

When the work is done, all companions assigned to the construction is returned without any selection process (just like "normal" jobs don't ask for who to return when there's only one candidate).

When emergency recalling a companion and there there is at least one companion left working on a construction mission, sum up the remaining time and dividing it over the remaining companions (so they will have longer before they're done). The last one removed will drop the job completely as previously.

Describe alternatives you've considered

  • Allow companions to be added to ongoing missions. Decided against it because it's messy with the "work day" conversion and it requires some new UI element to order it. To much work and complexity for something that ought to be of marginal utility.
  • Add one complete set of tools for each companions. This is NOT done currently: the code just checks that tools are available, but doesn't actually allocate them and return them. There is internal functionality for storing the tools in the companion mission data, but it's not used, at least not for construction. As issues with logic to collect and return tools is expected, the current approach of looking the other way is used, even if this means 100 companions could dig a trench with a single shovel.
  • Rejected any notion of trying to figure out how useful additional companions would be based on what's constructed, as that would be a rather complex issue.
  • Some other formula for how additional companions set on the task would scale. Some tasks would be faster with multiple workers (digging a trench), while others would benefit little or not at all (digging a well/a tower), while others would benefit greatly from 4 hands rather than two. Linear scaling seems to be good enough for a reasonable fit, especially since most camp construction blueprints are large enough that several companions could work on their own set of tiles in parallel.
  • Ignore it while waiting for camp construction to be morphed into construction zones. However, it seems there's quite a lot of waiting before that functionality has sufficient maturity and support to be used for camp blueprints. We can wait for that with this functionality in place.
  • Assign some arbitrary max number of companions that can be assigned to each construction mission. Decided the natural limit imposed by the number of companions available is probably better. If someone amasses 100 companions and gets a construction task done in minutes, more power to them (and a minor consolation prize when trying to manage a horde).
  • Changing the companion return logic so multiple companions can be announced at once.

Testing

Assigning one and two companions to camp and expansion upgrade tasks and verifying that things behave as expected when finalizing and emergency recall companions.

Additional context

Expansion task selection. No change in functionality, but note the time (2 days, 6 hours):
Screenshot (210)

Selection of a companion. Again, no change from the current functionality:
Screenshot (211)

Selection of the other companion. The prompt text is different (and there's no abortion message when not selecting a additional companion):
Screenshot (212)

The two companions are working on the construction: Note that there are two of them, and the time listed is half that of the original (single companion) time (but could be different due to work days conversions):
Screenshot (213)

The same job one day later:
Screenshot (214)

The same job after having emergency recalled one of the companions, with double the time remaining:
Screenshot (215)

A different time line where nobody was emergency recalled, and two days have passed from the start:
Screenshot (216)

Result when recalling the two companions from their completed task:
Screenshot (217)

And after dismissing the previous popup the second companion is also announced:
Screenshot (218)

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Mar 4, 2022
@PatrikLundell
Copy link
Contributor Author

PatrikLundell commented Mar 4, 2022

There's probably something wrong with a support tool, as I received an Email referencing "Pull Request Labeler: No jobs were run" with the following (although no failures were indicated in the list of checks yet):

Invalid workflow file: .github/workflows/labeler.yml#L12
The workflow is not valid. .github/workflows/labeler.yml (Line: 12, Col: 9): Unexpected value 'sync-labels'

Next support tool error: Apply Test Labels:

labeling
Unhandled error: HttpError: Label does not exist

General build matrix / Basic Build and Test (Clang 6, Ubuntu, Curses) (pull_request): Not sure which "All tests passed" sections with backtraces resulted in "14:01:05.069 INFO : Treating result as failure due to error logged during tests.
test exited with code 1
Error: Process completed with exit code 1."
Seemed to be something with map generation in both.

@Maleclypse Maleclypse added Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Player Faction Base / Camp All about the player faction base/camp/site labels Mar 4, 2022
@Maleclypse
Copy link
Member

There is a bugfix the labeler in play.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 4, 2022
@github-actions github-actions bot added the C++ label Mar 5, 2022
@PatrikLundell
Copy link
Contributor Author

PatrikLundell commented Mar 5, 2022

"Clang 12, Ubuntu, Tiles, ASan": failed due to some broken monster attack test:

Mods-(~[slow] ~[.],starting_items)=> cata_test is a Catch v2.13.7 host application.
Mods-(~[slow] ~[.],starting_items)=> Run with -? for options
Mods-(~[slow] ~[.],starting_items)=> 
Mods-(~[slow] ~[.],starting_items)=> Randomness seeded to: 1646472042
Mods-(~[slow] ~[.],starting_items)=> 
Mods-(~[slow] ~[.],starting_items)=> -------------------------------------------------------------------------------
Mods-(~[slow] ~[.],starting_items)=> monster_throwing_sanity_test
Mods-(~[slow] ~[.],starting_items)=> -------------------------------------------------------------------------------
Mods-(~[slow] ~[.],starting_items)=> ../tests/monster_attack_test.cpp:176
Mods-(~[slow] ~[.],starting_items)=> ...............................................................................
Mods-(~[slow] ~[.],starting_items)=> 
Mods-(~[slow] ~[.],starting_items)=> ../tests/monster_attack_test.cpp:230: FAILED:
Mods-(~[slow] ~[.],starting_items)=>   CHECK( damage_dealt.test_threshold( epsilon_threshold{ expected_damage, 2.5 } ) )
Mods-(~[slow] ~[.],starting_items)=> with expansion:
Error: Mods-(~[slow] ~[.],starting_items)=>   false

LGTM analysis: C/C++ tripped over its feet by timing out, as usual.

@PatrikLundell
Copy link
Contributor Author

Another win for the house in the test casino:

"LGTM analysis: C/C++" timed out as usual.

@kevingranade kevingranade merged commit 7354e9d into CleverRaven:master Mar 9, 2022
@PatrikLundell PatrikLundell deleted the multiple_constructors branch March 9, 2022 06:57
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 Crafting / Construction / Recipes Includes: Uncrafting / Disassembling 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.

For faction bases make it so you can send more people on construction mission to drastically cut time
3 participants