-
Notifications
You must be signed in to change notification settings - Fork 221
Update emigration.lua #735
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
Conversation
Fix for emigration.lua not working since the citizenship logic was changed.
myk002
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed this version of the code since it is clearer than the PR meant for master, but I'm not sure if we'll merge it. we don't intend to make another 0.47.05 release unless there is a severe issue with the current release, like a crash bug. I'll have to confer with the other devs whether there are any down sides to merging it into the v0.47.05 branch anyway, just so the improvements are saved.
please convert tabs to spaces so the formatter check passes.
emigration.lua
Outdated
|
|
||
| -- erase the unit from the fortress entity | ||
| for k,v in pairs(fort_ent.histfig_ids) do | ||
| if tonumber(v) == hf_id then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in what cases will this not already be a number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably none, yeah.
emigration.lua
Outdated
| end | ||
|
|
||
| -- erase the unit from the fortress entity | ||
| for k,v in pairs(fort_ent.histfig_ids) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while pairs likely still works, the semantics of ipairs are more appropriate. here and in all the loops below
emigration.lua
Outdated
|
|
||
| -- try to find a new entity for the unit to join | ||
| for k,v in pairs(civ_ent.entity_links) do | ||
| if v.type == 1 and v.target ~= fort_ent.id then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the meaning of 1? if it's an enum value, what's the enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's df.entity_entity_link_type.CHILD, the other one is df.entity_site_link_type.Claim
emigration.lua
Outdated
| then | ||
| if unit.flags1.merchant then table.insert(merchant_civ_ids, unit.civ_id) end | ||
| if unit.flags1.diplomat then table.insert(diplomat_civ_ids, unit.civ_id) end | ||
| --if unit.flags1.diplomat then table.insert(diplomat_civ_ids, unit.civ_id) end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to keep commented-out code. can remove
emigration.lua
Outdated
| for _, civ_id in pairs(merchant_civ_ids) do checkForDeserters('merchant', civ_id) end | ||
| for _, civ_id in pairs(diplomat_civ_ids) do checkForDeserters('diplomat', civ_id) end | ||
| checkForDeserters('wild', -1) | ||
| --for _, civ_id in pairs(diplomat_civ_ids) do checkForDeserters('diplomat', civ_id) end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
Fixed the things that were pointed out.
Removed excess whitespace
|
I think I did all the changes you suggested + it also passes the checks. |
|
I don't have any particular issues with making changes to the 0.47.05 branch (we have already done it in the other two repos), but I agree that it is unlikely to ever be released formally. |
|
Yeah. I figured that if people keep playing 0.47, they can always fetch updated scripts from github even if there are no new releases. I also figured that since the changes in 50 are so big, 0.47 might stay relevant longer than usual. |
|
No one's aware that we have a duplicate of this? (#736) What's to be done about it, then? |
It's the same fix to the other branch, because the v0.47 script doesn't work in 50 and vice versa. |
|
the build actions in this branch depend on an EOL version of ubuntu, which is why they aren't completing (I think). merging anyway since pre-commit.ci passes and no docs were changed. |
Followup to DFHack/dfhack#1784 (comment)
Fix for emigration.lua not working since the citizenship logic was changed.
It works, based on some testing by myself in 0.47.05. It can be tested relatively easy with stress-inducing scripts, such as this one I created while testing this: https://github.com/wsfsbvchr/dfhack-utils/blob/main/stress-tests.lua
Some edits might be needed, dunno.