4ian / GDevelop

🎮 Open-source, cross-platform 2D/3D/multiplayer game engine designed for everyone.
https://gdevelop.io
Other
11.24k stars 871 forks source link

json collaboration - file stability #6045

Open funjobie opened 11 months ago

funjobie commented 11 months ago

Is there an existing issue for this?

Describe the bug

When collaborating (we did a jam session with 2 people), there are some stability topics with the way the json files are currently handled. We already use the "multiple files" option, and were using git.

  1. While the sorting order is for the most part stable, we found instances of reordering

    JSON_stability
  2. We were struggeling with line endings. Despite both systems being windows, and despite us adding a .gitattributes file with *.json crlf we still found that the two systems behaved differently. In the end I am not really sure though if this was a user error with git settings or related to how gdevelop saves the json files (do you enforce a line ending style on write or do you use system settings?).

  3. Lastly, but this could be just a lack of experience with structuring projects. We found that quite often we would end up creating global objects, for anything from enemies, projectiles, tiles and so on, because we want to use them in all scenes. All of them are enumerated, including their behaviors with details, in one common json file therefore creating a synchronization point which needs resolving. Maybe this is already planned to be adressed by the "Custom Objects ("prefabs") " feature, but at least currently we spent ~25% of the time just merging stuff as it breaks the flow of working.

Steps to reproduce

I am unsure if its possible to reproduce such topics without at least two systems or users

GDevelop platform

Desktop

GDevelop version

5.3.181 (verified that both team members had this version)

Platform info

*OS (e.g. Windows, Linux, macOS, Android, iOS)* >Windows *OS Version (e.g. Windows 10, macOS 10.15)* >Windows 10 *Browser(For Web) (e.g. Chrome, Firefox, Safari)* > *Device(For Mobile) (e.g. iPhone 12, Samsung Galaxy S21)* >

Additional context

No response

AlexandreSi commented 11 months ago

Hi @funjobie, thanks for taking the time to suggest some things to improve GDevelop, and especially the "Multiple files" saving features.

Here is what I understood:

  1. Objects (or maps) are not serialized with a stable way: sometimes the attributes are sorted alphabetically, sometimes not. This makes using git painful since there was no changes in some items (behaviors for instance). I think this needs some investigation to know if it's doable without impacting performances (to make sure the save time stays short).
  2. You encountered an issue regarding line endings. Regarding this one, I'm not sure GDevelop is to blame. I think we're using the same method anywhere so I guess that there should be consistency between all the files. But I'm maybe mistaken.
  3. Global objects are stored at the project level, that is indeed stored in a single file, even with the "Multiple files" option. Custom objects won't fix this since they will still belong to either a scene or the project. What were the changes you were doing in parallel that you hoped would not trigger conflicts? I'm not sure it's a problem of how global objects are stored but more a problem about how to work at the same time on the same items.
funjobie commented 11 months ago

For point 2. Yeah entirely possible our local setup and/or git contributed. But yes we saw this across all json files. i believe the initial GDevelop project i created was using lf and when my teammate cloned it and saved changes, the changed files where crlf, so showing up as changed. For point 3. From my perspective there are 2 use cases:

A is the one that irks me more. I was expecting a json file per global object so that we can then really split up work. Its actually not so much conflicts but just rebasing. For example if someone changes the collision mask on object A and another changes the PlatformerObjectBehavior details of another one (or adds a variable to the object), both changes affect the same file. this can be easily auto-merged because its in different blocks in the json, but still needs a careful eye because merges might not be perfect. In c++/c# land where i come from, you would just have different files for classes and can for the most part work fully independent from each other - once you have agreed on the interface.

B is what today causes actual merge conflicts, because both parties create a new block in the json. I think the current central list for layouts/externalEvents/eventsFunctionsExtensions/externalLayouts makes sense. The resource list and global object list, at least to me, are less ideal. Personally, I'd probably favor collecting those from seperate folders just based on tags (e.g. obj1.json / obj2.json etc each with a json tag "GlobalObject", which the importer would just look for). That way new ones can be added by independent developers. But that might be too drastic :)

4ian commented 11 months ago

Hopefully a good first step would be too fix the stability of the serialization (which has in theory no impact on functionality, and I can't see any argument against it). We use RapidJSON for outputing JSON, or sometimes JS builtin JSON.stringify, it might need a bit of investigation because it's possible one or the other is causing the issue.

Then we can talk about files splitting, there is maybe something to do for objects. But this is a larger/more risky change (because we're forced to support this forever then) so better do the low hanging fruit of fixing serialization key order first :)

funjobie commented 11 months ago

Yeah i agree, the stability improvements would already help somewhat short term. Could also be that other/bigger teams found better proposals or best practices, so nothing requested in a hurry :)