Skip to content

New Feature: autotraining #1411

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

Merged
merged 74 commits into from
Aug 17, 2025
Merged

Conversation

realSquidCoder
Copy link
Contributor

Code for dwarves to hit the gym when they yearn for the gains. Assigns dwarves to a military squad until they have fulfilled their need for Martial Training

Code for dwarves to hit the gym when they yearn for the gains.
Assigns Dwarves to a military squad until they have fulfilled their need for Martial Training
@realSquidCoder realSquidCoder marked this pull request as draft March 2, 2025 04:22
@realSquidCoder realSquidCoder marked this pull request as ready for review March 2, 2025 18:50
@chdoc
Copy link
Member

chdoc commented Mar 6, 2025

Here are some observations:

  1. I'm not particularly fond of the name gym, because it makes me think of using screw pumps to increase strength. Maybe just call it martial-training or martial-arts-class
  2. I think the squad should just be chosen from a list, instead of requiring the user to name a squad in a particular way. I think that squads are intended to use DF names, and I would not be surprised if the ability to just input strings would disappear. Maybe, if there is only a single squad that does not have a uniform set, default to that.
  3. I would like an option to not automatically replace the squad leader. This way you could actually put a decently skilled teacher in charge of the squad. Also, squad leaders are nobles and this would avoid at least some of the historical figure noise.

@chdoc
Copy link
Member

chdoc commented Mar 6, 2025

  1. It would be nice to have an option to automatically exclude holders of relevant positions (e.g. the manager ...)

@realSquidCoder
Copy link
Contributor Author

1. I'm not particularly fond of the name `gym`, because it makes me think of using screw pumps to increase strength. Maybe just call it `martial-training` or `martial-arts-class`

Ok thats not a terrible idea. the original name was just needs-training so maybe martial-training works

2. I think the squad should just be chosen from a list, instead of requiring the user to name a squad in a particular way. I think that squads are intended to use DF names, and I would not be surprised if the ability to just input strings would disappear. Maybe, if there is only a single squad that does not have a uniform set, default to that.

the reason you need to make a squad (tho i want to make the script make it's own squad in a later stage) is because it lets you give them the proper schedule and teacher leader.

3. I would like an option to not automatically replace the squad leader. This way you could actually put a decently skilled teacher in charge of the squad. Also, squad leaders are nobles and this would avoid at least some of the historical figure noise.

its not supposed to replace the leader, have you seen it doing that?

@realSquidCoder
Copy link
Contributor Author

  1. It would be nice to have an option to automatically exclude holders of relevant positions (e.g. the manager ...)

you can set up ppl to be ignored but auto ignoring ppl is not a bad idea either. who all should be ignored you think?

@realSquidCoder
Copy link
Contributor Author

actually, because i intend this to join the automation tab, perhaps autotraining would be a better name

- Clean up documentation
- Add option to change squad name.
- persist the enabled state, the threshold, and the squad name.
- fixed findNeed function
- renamed script to `autotraining`
- made the ignore flag more clear and more changable
- fixed 1 sided military link in `addTraining`
Also tell the user when data was persisted (mostly for debugging)
and update the docs to account.
@realSquidCoder realSquidCoder changed the title New Feature: gym New Feature: autotraining Mar 12, 2025
improvements to `autotraining`:
- fix the argument error
- avoid the double execution of the loop when enabling
- consistently only count ignored units when they would otherwise qualify for training
- allow enabling the tool from within `gui/autotraining`
- sort the list of training candidates, so that the most needed candidates are preferred for training
- move the argument handling out of the `start` function

Co-Authored-By: Christian Doczkal <[email protected]>
@realSquidCoder
Copy link
Contributor Author

pre-commit.ci autofix

@ab9rf
Copy link
Member

ab9rf commented Aug 16, 2025

Needs a changelog, otherwise I think this is ready to merge.

Copy link
Contributor

@SilasD SilasD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see anything bad enough to justify delaying a merge.

I have some quibbles with command-line processing and with the enable/disable code.
enable autotraining
disable autotraining
It doesn't appear that these command-line commands will work. (not verified.)

I somewhat question the need for autotraining as a separate script. It seems to me that it could be rolled into gui/autotraining.

@ab9rf
Copy link
Member

ab9rf commented Aug 17, 2025

I somewhat question the need for autotraining as a separate script. It seems to me that it could be rolled into gui/autotraining.
i'm sorry, gui/autotraining? i looked for this and could not find it. to what are you referring?

@chdoc
Copy link
Member

chdoc commented Aug 17, 2025

This pull-request adds both: autotraining as the tool that performs the actual work and can also be (partially) configured using the command-line and gui/autotraining a graphical interface to configure squads and ignored units. If we want to follow the convention that tools that open windows are invoked as gui/something and tools that perform automation are enabled as enable something we need two commands and therefore two files.

@ab9rf
Copy link
Member

ab9rf commented Aug 17, 2025

Is everyone ok with a squash merge on this one?

@chdoc
Copy link
Member

chdoc commented Aug 17, 2025

During one of my last sessions I noticed something that I cannot yet explain:

autotraining: 1 training, 12 waiting, and 3 excluded units with training needs

The expected behavior would be 9 training units and 4 waiting units, no?

@realSquidCoder
Copy link
Contributor Author

realSquidCoder commented Aug 17, 2025

@chdoc frack its still doing that? as far as i know that thats a miscount bug not a wrong effect bug, i thought i had fixed all of them tho...

edit: wait no, what i misread that... what is it doing there? (still think we can merge this to get more testers)

@realSquidCoder
Copy link
Contributor Author

@ab9rf everything seems to have passed so if the others have no issues im cool with a squash and merge. (and of course i'll be around to fix any bugs that are found in the next release)

@ab9rf ab9rf merged commit a02935d into DFHack:master Aug 17, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in 51.11-r2 Aug 17, 2025
@realSquidCoder realSquidCoder deleted the sci-gym-script branch August 17, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants