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

Fix plArmatureLODMod writing PRC Meshes tag twice #285

Merged

Conversation

dgelessus
Copy link
Contributor

The base class plArmatureModBase already writes the Meshes tag, so plArmatureLODMod shouldn't write it a second time.

Copy link
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@zrax zrax left a comment

Choose a reason for hiding this comment

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

Ugh. These appear to be duplicated because the stream I/O also has duplicate readers and writers for them... There's nothing we can do about the stream format without breaking existing PRPs, but yeah, it makes sense to at least clean up the PRC output.

@zrax zrax merged commit ebb8db0 into H-uru:master Mar 29, 2024
8 checks passed
@dgelessus dgelessus deleted the fix_plarmaturelodmod_prc_duplicate_meshes branch March 29, 2024 00:14
@dgelessus
Copy link
Contributor Author

These appear to be duplicated because the stream I/O also has duplicate readers and writers for them...

Actually, nothing is duplicated in the stream data either - but
the code is really wacky here. There are two places that read the meshes (plArmatureModBase::read and plArmatureLODMod::read), but only the latter is actually executed. Note that (for Uru PRPs) plArmatureMod::read doesn't call the base class method plArmatureModBase::read, but instead skips one class up the chain to plAGMasterMod::read. So one of the two places where the meshes are read is always skipped. The same thing happens when writing.

The trouble was that the PRC read/write code replicated one half of this wacky logic (two classes reading/writing the same field), but not the other half (skipping one of those two implementations).

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.

3 participants