deepnight / ldtk

Modern, lightweight and efficient 2D level editor
https://ldtk.io
MIT License
3.37k stars 184 forks source link

Alternative method for writing to `.ldtkl` files #681

Open Trouv opened 2 years ago

Trouv commented 2 years ago

Currently, when saving a project with external levels, it seems the external level files are written to in a strange way. For whatever reason, external level files become unavailable for a moment while the project is being saved. This may be because they're removed first and then new files are written in their place, or maybe because they're just being written to directly (as opposed to being written to a temp file first which is then moved on top of the original files).

Either way, this can cause trouble for other programs reading those files at the same time. For example, in bevy, you can hot reload assets so changes made to the assets in the file system get reloaded into a currently-running game. My importer treats .ldtkl files as assets, but I can't hot reload these files since bevy's filesystem watcher encounters an error in the period that these files are being written to by LDtk, and then doesn't detect the changes after the error. This is not an issue for the main .ldtk file, even in non-external-level projects. Here's the corresponding issue for bevy https://github.com/bevyengine/bevy/issues/3359.

While I think bevy should be able to detect changes in this situation, I also think the way LDtk writes these files could be less disruptive.

Cammin commented 2 years ago

This issue could be possible for my Unity importer as well actually. But I have YET to encounter a problem with it. I will confirm if I get an issue with this after testing.

For context, Unity creates an associated sibling .meta file for every file+folder under the unity project's directory. The meta files are used to track GUIDs for reference purposes, and can also contain custom serialized field information. So meta files are an important part of unity.

For unity importers like model importer, texture importer, and LDtk importer, in this case, there are many configuration fields in the inspector UI that can affect the import result. Things like pixels per unit, entity replacements, collision configuration, etc, which are all serialized in the metafile.

The key here is that if a unity asset has its source file deleted, then the meta file gets deleted alongside it, and regenerates a default metafile. (The metafile is only deleted if unity realizes the source file is gone, so the editor would need to be the active selected Windows application)

This ends my context, so with that in mind, this means that if the .ldtk file was temporarily deleted, then issues could arise. But I don't think that's the case because this topic talks about separate level files.

In the context of separate level files for my case, there is no important serialized data into the level's metafiles, so I would not have a glaring issue if the metafile was regenerated. Still, I believe that regenerating a meta file because a level file was temporarily deleted could have other unexpected issues like a regenerated asset GUID, so I believe this would be a good issue to have fixed.

I will do some tests and see if I can make a metafile get regenerated by clicking into unity while LDtk is saving separate level files to confirm my theory. I will notify you back here if my issue is true 🙂


Edit: I've done some tests, and I don't seem to have any issues with checking into unity while saving levels, so I should be okay for this.

deepnight commented 2 years ago

Thanks for pointing this out. A general rule of thumb when dealing with files updated by external apps is to make sure you don't reload if:

  1. your own app is not focused and active (no background update)
  2. always wait a little bit to make sure the file is final and ready,
  3. handle failure and allow multiple retries.

I did have a similar issue with Aseprite files, which LDtk actively watches. I did all of above and everything went smoothly.

Anyway, that being said, I agree some changes could be done in the way LDtk handles saving, to be a little bit more "atomic".