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

Fixed XML deserialization of Curve type #5494

Merged
merged 4 commits into from Mar 1, 2017

Conversation

Projects
None yet
3 participants
@Jure-BB
Copy link
Contributor

commented Feb 22, 2017

Fixes issue #5492.

  • Changed order of properties inside Curve class so that order in serialization/deserialization matches XNA's
  • Added CurveKeyCollectionSerializer that serializes/deserializes CurveKey collection packed like XNA
  • Added unit tests
@KonajuGames

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2017

Changing the order of fields is not reliable because the C# compiler can re-order fields and properties if it needs to. The is an attribute to prevent re-ordering, but if the XNA class didn't have it then we shouldn't.

@KonajuGames

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2017

New source files need to be added to the Protobuild definition files in the Build directory.

@Jure-BB

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2017

New source files need to be added to the Protobuild definition files in the Build directory.

New *cs files that I added to the project?

@Jure-BB

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2017

Changing the order of fields is not reliable because the C# compiler can re-order fields and properties if it needs to. The is an attribute to prevent re-ordering, but if the XNA class didn't have it then we shouldn't.

I tried to change order of elements in xml and open it with XNA Curve Editor tool and it fails to open it. So XNA depends on order of elements inside xml. I can't find any serializer specific for Curve class inside XNA so I have no idea how they do it then.

EDIT: Monogame depands on the order of fields for every class that is serialized/deserialized by ReflectiveSerializer, not just Curve class. I just made Curve class "compatible" with this design. XNA has same design issue.

Unknown added some commits Feb 22, 2017

@tomspilman tomspilman added this to the 3.6 Release milestone Mar 1, 2017

@tomspilman tomspilman changed the title Support for Curve xml deserialization that was created by XNA Fixed XML deserialization of Curve type Mar 1, 2017

@tomspilman

This comment has been minimized.

Copy link
Member

commented Mar 1, 2017

Thanks @zigzag312 ... merging!

@tomspilman tomspilman merged commit 1569375 into MonoGame:develop Mar 1, 2017

5 checks passed

Build Mac, iOS, and Linux Finished TeamCity Build MonoGame :: Build Mac : Running
Details
Build Windows, Web, Android, and OUYA Finished TeamCity Build MonoGame :: Build Windows : Running
Details
Package Mac and Linux Finished TeamCity Build MonoGame :: Package Mac and Linux : Running
Details
Package Windows SDK Finished TeamCity Build MonoGame :: Package Windows : Running
Details
Test Windows Finished TeamCity Build MonoGame :: Test Windows : Tests passed: 1075, ignored: 11
Details

rds1983 added a commit to rds1983/MonoGame that referenced this pull request Jun 6, 2017

Fixed XML deserialization of Curve type (MonoGame#5494)
* Support for Curve xml deserialization that was created by XNA

* Added new source files to Protobuild definition files

* Added new test xml file to Protobuild definition file

* Removed C# 6.0 use
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.