helper functions for creating and assigning jobs

[chdoc]

I finally got around to following up on this discussion concerning unitst::have_unbailable_sp_activities

The motivation for this is that we now have several tools that create jobs and immediately assign them to dwarves (idle-crafting, autocheese, immortal-cravings). This has led to a bunch of code duplication and cross-importing between those tools. I would like to clean this up a bit by putting some of the frequently used functionality on a more solid footing and moving it into the C++ libraries.

@ab9rf It would be nice if you could take a look at unbailableSocialActivity and see if it needs any corrections or extensions for our purposes.

[SilasD]

Wrong Discord link? I don't see anything relevant near that post. Now I see it talking about unitst::have_unbailable_sp_activities, did I just not notice that? Never mind.

Edit: I don't think I'd use the proposed job creation code. I want fine-grained control, and I want to add the job to the world as the last step, in case something goes wrong during my job creation.

If I were ever to assign units to jobs, I would probably like to use this code in order to handle all cases, including cases I'm not aware exist.

At a glance, I don't like the name isJobAvailable(), something like isAvailableForWork() seems closer to the intent. preserve_social also doesn't feel expressive to me. dont_interrupt_activities ?

Same for unbailableSocialActivity(), I don't like 'bail'. hasUninteruptableSocialActivity() ?

Or just fold these tests into addWorker() ?

I might like to have a findBestAvailableWorker(job), or simply addBestWorker(job). This would require some analysis of the job. Hmmm, in the case of a workshop job, this would need to check if the workshop is assigned to a single worker.

Consider renaming assignToWorkshop() as assignToBuilding() and only testing the number of jobs for the workshop/furnace case,

[chdoc]

Edit: I don't think I'd use the proposed job creation code. I want fine-grained control, and I want to add the job to the world as the last step, in case something goes wrong during my job creation.

Usually, I tend to check all the conditions that are necessary to create a job before attempting the creation. For tools that immediately attach workers, that includes looking whether there is an available worker. So it makes a lot of sense to keep the test separate from addWorker.

There isn't really much that can "go wrong" in setting job parameters and adding general references. In fact, if memory serves me right, DF links jobs into the world as part of the constructor. That is of course very restrictive because it doesn't allow you to store and pass around (unlinked) copies of jobs.

I might like to have a findBestAvailableWorker(job), or simply addBestWorker(job). This would require some analysis of the job.

If the goal is to get work done, then the preferred way is probably you just post the job and let the game handle the rest. addWorker is mainly for the case where you want a unit to do something that will satisfy one of its needs.

[ab9rf]

I would prefer that these be constexpr but std::bitset's operators aren't constexpr until C++23, so I'm going to put this here as a vague reminder to reconsider if/when we move to C++23 compliance

[chdoc]

I share your sentiment. This is why I left a comment a few lines above the lines you selected.

[ab9rf]

The new methods need to be documented in docs/dev/Lua API.rst

Otherwise, looks good to me

[ab9rf]

Same for unbailableSocialActivity(), I don't like 'bail'. hasUninteruptableSocialActivity() ?

"bailable" is literally Tarn's term for this: this function is a (fairly faithful) reimplementation of a function in Bay12 code called have_unbailable_sp_activities and I'm fairly certain the reason @chdoc called it that is because I mentioned that that was Tarn's name for the function during the reverse engineering process that led to this implementation. I can see value in preferring Bay12 names for concepts, and thus I think we can live with using "bailable" instead of "interruptible". Not to mention people seem to have a lot of trouble spelling "interruptible" as it is.

In fact, if memory serves me right, DF links jobs into the world as part of the constructor.

This is correct. DF just doesn't have a concept of a job that isn't on the master job list. While it might be nice to have a job that isn't linked in some situations, this doesn't agree with how DF conceptualizes jobs. While I don't think the game will actively misoperate if a job that isn't linked gets referred to, I wouldn't count on this and it's generally best that we not blithely violate the assumptions of the core game engine without carefully considering what the consequence of such violations are. As such, I'm not in favor of having a utility function that creates jobs but doesn't link them, without a specific demonstration of a meritorious use case for such a utility function. If the only situation you can come up with is "something could go wrong during job construction", the solution to that is "verify that all of your preconditions are satisfied before you actually create the job".

[chdoc]

The new methods need to be documented in docs/dev/Lua API.rst

Presumably you mean that functions should be exported to lua and documented in docs/dev/Lua API.rst. I didn't want to do the lua export before knowing which functions will actually be included and which will not.

Looking at the signatures, I think that everything other than the getFocusPenalty overload for sets can be exported without shims. To keep things simple, I would only export the one that takes a single need type.

[ab9rf]

above and beyond, well done

[ab9rf]

omit the e here, since you don't use it