Skip to content

Conversation

@wsfsbvchr
Copy link
Contributor

@wsfsbvchr wsfsbvchr commented Jun 8, 2023

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.

Fix for emigration.lua not working since the citizenship logic was changed.
Copy link
Member

@myk002 myk002 left a 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
Copy link
Member

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?

Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

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?

Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

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

remove

wsfsbvchr added 2 commits June 8, 2023 23:58
Fixed the things that were pointed out.
Removed excess whitespace
@wsfsbvchr
Copy link
Contributor Author

I think I did all the changes you suggested + it also passes the checks.

@lethosor
Copy link
Member

lethosor commented Jun 9, 2023

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.

@wsfsbvchr
Copy link
Contributor Author

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.

@silverwing235
Copy link

No one's aware that we have a duplicate of this? (#736) What's to be done about it, then?

@wsfsbvchr
Copy link
Contributor Author

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.

@myk002
Copy link
Member

myk002 commented Jun 10, 2023

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.

@myk002 myk002 merged commit a1f253f into DFHack:v0.47.05 Jun 10, 2023
@wsfsbvchr wsfsbvchr deleted the patch-2 branch June 20, 2023 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants