Skip to content
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

itemst vmethod update & housecleaning #653

Merged
merged 3 commits into from
Sep 10, 2023
Merged

Conversation

ab9rf
Copy link
Member

@ab9rf ab9rf commented Sep 8, 2023

identified a significant number of anonymous vmethods in itemst

also cleaned up the index numbers in the xml, they were badly out of whack

<vmethod ret-type='int32_t' name='calcBaseUniformScore'/>
<vmethod><int32_t/><int32_t/><int32_t/></vmethod>
<vmethod ret-type='int32_t'> <int32_t name='race_id'/><int32_t name='caste_id'/><int32_t name='maximum'/></vmethod>
Copy link
Member

Choose a reason for hiding this comment

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

no name for this one?

Copy link
Member Author

Choose a reason for hiding this comment

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

nope. don't know what it does

df.items.xml Outdated
@@ -623,46 +631,60 @@
<pointer name='u' type-name='unit'/>
<pointer name='j' type-name='job'/>
</vmethod>
<vmethod><int32_t/><int32_t/><int32_t/><int32_t/><int32_t/><int32_t/><int32_t/><int32_t/><int32_t/><int32_t/><int32_t/><int32_t/></vmethod>
<vmethod><int32_t/></vmethod>
<vmethod name='addImprovements'>
Copy link
Member

Choose a reason for hiding this comment

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

addImprovement? I only see one

Copy link
Member Author

Choose a reason for hiding this comment

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

the code may be able to add more than one improvement, and it is parallel with the subsequent method which definitely can add multiple improvements

Copy link
Member Author

@ab9rf ab9rf Sep 10, 2023

Choose a reason for hiding this comment

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

ok, i reviewed the one non-trivial implementation and it only adds one improvement, so we'll drop the 's'

i originally thought it could add two in some cases, but they're on opposite side of a test and i am pretty sure there's no code path where both get added

@myk002 myk002 merged commit 4097a59 into DFHack:master Sep 10, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants