Bug #1618
openAdd-to-city always adds specialists
0%
Description
Even when the unit has no "specialist" defined, add-to-city creates default specialists, not field workers.
Files
Updated by Alexandr Ignatiev about 1 year ago
Is it a bug at all? Possibly we don't want our carefully placed workers rearranged when we add one more citizen. Here we know where the newly added citizen is. This is the behavior that existed for ages. Maybe though we could respect the city setting "new citizens will be...".
Updated by Marko Lindqvist about 1 year ago
Alexandr Ignatiev wrote in #note-1:
This is the behavior that existed for ages.
Oh. I thought it was new with your recent changes. Guess I haven't been using add-to-city too much previously (now been testing foodbox behavior when city grows by different means)
Updated by Dean Brown about 2 months ago
- File 1618_Main.patch added
- File 1618.patch added
See also #2080
Could call this a feature request rather than a bug. That city setting is not used when a city grows normally, nor when it gets migrated to - in those cases auto_arrange_workers() gets called. I think that's the best way to go. The current code usually leaves the city with a food deficit, and if the user doesn't click on the city and re-assign the new citizens the city will lose pop, possibly on the next turn.
Updated by Dean Brown about 2 months ago
- File 1618.patch 1618.patch added
Could target this to 3.2.6
Updated by Marko Lindqvist about 1 month ago
- Assignee deleted (
Alexandr Ignatiev)
Updated by Marko Lindqvist 13 days ago
The patch seems wrong for main branch. When the unit does have "specialist" defined, it should be respected (what otherwise is the point of that feature?)
Updated by Dean Brown 12 days ago
- File 1618_Main.patch added
Yeah that Main patch was bad - not understanding the new superspecialist feature. I'm still a bit unclear about the new
struct specialist spec_type; / affects only founding and adding to cities */in struct unit_type. It looks to me like it is only used for superspecialists, so I think it can be ignored if is_super_specialist(act_utype->spec_type) returns FALSE. Maybe I'm wrong?
Updated by Marko Lindqvist 12 days ago
- Category set to Server
- Target version set to 3.3.0
Updated by Marko Lindqvist 12 days ago
Dean Brown wrote in #note-9:
Yeah that Main patch was bad - not understanding the new superspecialist feature. I'm still a bit unclear about the new
struct specialist spec_type; / affects only founding and adding to cities */
in struct unit_type. It looks to me like it is only used for superspecialists, so I think it can be ignored if is_super_specialist(act_utype->spec_type) returns FALSE. Maybe I'm wrong?
As far as I know, nothing prevents one from setting regular specialists as unit spec_type, even though the feature was originally meant for superspecialists. We should do the right thing also when spec_type is set but not superspecialist.
Updated by Dean Brown 12 days ago
Looks to me like the spec_type gets defaulted to DEFAULT_SPECIALIST in load_ruleset_units(), and that code won't allow setting it to NULL. How would you un-set the unit spec_type? Is it possible to set it to none? I want to be able to have "add to city" not always add specialists. Maybe load_ruleset_units() should allow setting spec_type to NULL if there's no "specialist" entry for a unit, then the code in city_add_unit() could test for that.
Updated by Marko Lindqvist 12 days ago
Dean Brown wrote in #note-14:
Looks to me like the spec_type gets defaulted to DEFAULT_SPECIALIST in load_ruleset_units(), and that code won't allow setting it to NULL. How would you un-set the unit spec_type? Is it possible to set it to none? I want to be able to have "add to city" not always add specialists. Maybe load_ruleset_units() should allow setting spec_type to NULL if there's no "specialist" entry for a unit, then the code in city_add_unit() could test for that.
I didn't know ruleset_load() sets it even if it's not set in the ruleset. Can you open a new (future) ticket about those changes needed to make regular specialist behave correctly? I guess this one should go forward as is for now.
Updated by Marko Lindqvist 10 days ago
- Status changed from New to In Review
- Assignee set to Marko Lindqvist
Updated by Marko Lindqvist 2 days ago
- Status changed from In Review to In Progress
- Assignee changed from Marko Lindqvist to Dean Brown
I got an autogame sanity check failure "in auto_arrange_workers() ... citizens not equal [size], repairing ...", and this patch seems likely reason. You make the new citizen nothing (neither worker nor specialist) before calling auto_arrange_workers().
When sanity checking is enabled, there's this "repairing" part, but in a release version new citizen would not be made anything by that either.
Seen on the main branch.
Updated by Marko Lindqvist 2 days ago
Marko Lindqvist wrote in #note-18:
I got an autogame sanity check failure "in auto_arrange_workers() ... citizens not equal [size], repairing ...", and this patch seems likely reason.
Managed to reproduce, and confirmed that it's the new call from city_add_unit()