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

Partial Tiled 1.8 Support #151

Merged
merged 3 commits into from
Nov 12, 2023
Merged

Partial Tiled 1.8 Support #151

merged 3 commits into from
Nov 12, 2023

Conversation

Karamu98
Copy link
Contributor

  • Properties of the type "class" can now be parsed, a reference to the users "propertytype" is now also stored if set
  • "class" type properties now recursively load child properties

This partially implements this change log.
I threw this together pretty quick but I'm happy to make changes if this isn't good enough right now, I'm not a fan of the recursive vectors for example but it got the job done for me!

- Properties of the type "class" can now be parsed, a reference to the users "propertytype" is now also stored if set
- "class" type properties now recursively load child properties
@fallahn
Copy link
Owner

fallahn commented Nov 12, 2023

Thanks! I'll have a good look ASAP

@Karamu98
Copy link
Contributor Author

Is this just a missing include for linux? I can pop that at the top of the file with a problem or is there a better place? Sorry never built for linux before!

@fallahn
Copy link
Owner

fallahn commented Nov 12, 2023

Yeah - looks like it's just missing the #include <memory> - not uncommon for different platforms. In fact I'm surprised it's not missing on mac too (it can be quite fussy). I usually keep a VM handy for this sort of thing, but if you prefer to update and push to your fork the CI should automatically kick in and re-run the checks.

@fallahn
Copy link
Owner

fallahn commented Nov 12, 2023

Looking more closely, I wonder if the shared_ptr is even needed? m_classValue should work just as well as a std::vector<Property> , I think. Perhaps if there is a clear need for there to be only a single instance of a class member then it would make sense (although even then probably fine to use references to a unique_ptr) - but as copying a Property copies all the other possible types it could contain, then I don't see why copying its class type member would be a problem. If you see what I mean?

@Karamu98
Copy link
Contributor Author

That's a good point, I reactively threw it onto the heap but that really isn't needed here! I'll get that changed:)

@fallahn
Copy link
Owner

fallahn commented Nov 12, 2023

Nice work - thanks! 😁

@fallahn fallahn merged commit a534fe3 into fallahn:master Nov 12, 2023
9 checks passed
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.

None yet

2 participants