TeamAtomECS / AtomECS

Cold atom simulation code
GNU General Public License v3.0
46 stars 12 forks source link

Serdeoutput - output for custom components and entities #47

Closed MauriceZeuner closed 3 years ago

ElliotB256 commented 3 years ago

@MauriceZeuner looks like you have some conflicts!

MauriceZeuner commented 3 years ago

I know how to resolve conflicts when I merge, but how do I do it here in the pull request? Does it need to be done online? I can view the conflicts, but I can't edit anything.

Or do I just keep committing to the branch until they're resolved?

ElliotB256 commented 3 years ago

update the branch so that it can be merged in to the current master. You might have to edit the PR to include your new commits.

MauriceZeuner commented 3 years ago

okay, just done that. How do I update the pull request? (it seems it already did so automatically)

ElliotB256 commented 3 years ago

Tried to look just now if there is something similar to queries in specs, but nothing as nice :(

MauriceZeuner commented 3 years ago

Just realized that I hadn't pushed the xyz files to this branch yet. I corrected that now.

MauriceZeuner commented 3 years ago

happy to merge?

MauriceZeuner commented 3 years ago

@ElliotB256 If there are no further objections, I'm going to merge it so I can resolve the conflicts with the dipole PR (because it already contains the new serdeoutput), okay?

ElliotB256 commented 3 years ago

I didn't have time to finish the review yet, please wait.

ElliotB256 commented 3 years ago

@MauriceZeuner

The PR contains an XYZPosition impl for Position and other XYZ system stuff - I don't think these systems and functionality are needed now that the more general serde impls are in? If you want them to remain, can you make a case for them? Otherwise, I think we should delete the XYZ stuff and only support Text, Binary, and SerdeJson (and other variants in the future).

MauriceZeuner commented 3 years ago

yes! .xyz is the only format that allows to directly import them into Blender and generate nice animations from them. I think that's really nice and I would appreciate it if we kept the functionality.