Skip to content

Breaking Change: Fix objects with the same container path - #172

Closed
t-wy wants to merge 2 commits into
K0lb3:masterfrom
t-wy:master
Closed

Breaking Change: Fix objects with the same container path#172
t-wy wants to merge 2 commits into
K0lb3:masterfrom
t-wy:master

Conversation

@t-wy

@t-wy t-wy commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

Addressing the issue #45, referenced PR #134 and AssetStudio's original implementation.

Reimplemented the array of KeyValuePair (multi-dict) structure in AssetStudio as a list-of-tuple structure to address the issue of multiple objects with the same container path when implemented as dict, such that all objects return the expected .container str value instead of None.
The tuple ((path, obj)) may be changed to a list ([path, obj]), a dict ({"path": path, "obj": obj}), or a new class structure (.path = path, .obj = obj) if it is intended to be mutable.

Breaking Part: All .items() call to .container or .m_Container is no longer needed as it is an array instead of a dict.
To avoid the compatibility issue, a list wrapper class with an .items() method that returns the list may be used in all container properties.

t-wy added 2 commits March 12, 2023 02:43
Reimplement the array of KeyValuePair (multi-dict) structure in AssetStudio as a list-of-tuple structure to address the issue of multiple objects with the same container path when implemented as dict.

Breaking Part: All .items() call to .container or .m_Container is no longer needed as it is an array instead of a dict.
@K0lb3

K0lb3 commented Apr 5, 2023

Copy link
Copy Markdown
Owner

Sorry for the late reply.
I'm aware of the issue and refrained from fixing it because of the issue you mentioned - that it would be a fatal breaking change.

I "fixed" it in a test branch some time ago without breaking the normal usage pattern while still allowing it to pass through all keys and values.
UnityPy@fsspec - UnityPy/files/SerializedFile.py

@t-wy

t-wy commented Apr 10, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the reply. I see the concern, especially when it comes to the compatbility issues.
Seems like the workaround wrapper provided is within my expectations. Looking forward to the release than :)
(Meanwhile I can still take some monkey patches to handle my own usages :))

@K0lb3

K0lb3 commented Jun 5, 2023

Copy link
Copy Markdown
Owner

implemented by #186

@K0lb3 K0lb3 closed this Jun 5, 2023
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.

2 participants