adventuregamestudio / ags

AGS editor and engine source code
Other
702 stars 160 forks source link

Editor: change Sprites storage in the game project to open format #1281

Open ivan-mogilko opened 3 years ago

ivan-mogilko commented 3 years ago

Problem

For historical reasons sprites in AGS game project are kept compiled in a single file (aka "sprite file"). Editor saves them there on import and reads them back from there when having to use these sprites displaying a game scene. When compiling the game this file is simply added to the data package as-is, and this is it's only benefit. But this has a number of disadvantages:

Solution summary

Questions and further suggestions follow...

Project storage

Need to define the subfolder to store sprites. "Sprites"?

Personally I doubt if subfolders within "Sprites" should correspond to the folders in the editor's sprite manager. IMO more no than yes. The imported sprites do not 1:1 correspond to the source images, user may import multiple sprites from one image file, even from one tile rect with different settings. Same image may be imported as multiple placeholders too. It may be imagined that at some point editor will be able to display both source images and imported sprites in separate windows - as separate entities.

Importing sprites

On a sprite import editor checks whether the source image is withing "Sprites" project folder, if not then copies it there. If we support subfolders, images cloned by the editor should probably go into a special "unorganized" subfolder. Editor will also have to prevent file name conflicts.

Alternatively we may restrict sprite import to only files that were placed into "Sprite" folder by user. Pros: user will have full control on how files are called, there's no need to guess how and where to clone files. Possibly less confusion about how import works. Cons: possibly extra work for users if they import from other locations.

What to do with "import from clipboard" option? If supported then it must create a new source image file in the project folder.

Upgrading from older project

Existing AGS project has:

Let's suppose that the priority should be given to import original images whenever possible. If they cannot be found, then we may optionally export sprites from the compiled sprite file and save them in "Sprites" folder.

In any case, what to do if there's already a "Sprites" folder created by user and containing some data?

If we are exporting from the spritefile, we will have to deal with possible duplicate image sources and sprites cut out from the middle of an image. E.g. there may be multiple sprites referencing one source image. In such case should we try to reconstruct original image judging by the sprite tile offsets & sizes? Or should we export each sprite to its own image file and adjust source references and import settings? Should the program ask for permission to export from the spritefile to let user opportunity to restore original images, or just warn and go on with it?

Sprites in the Editor's memory

Hypothetically, Editor may reuse existing sprite cache in C++ code with some adjustments. If it's preferable to move away from using C++ code internally in the Editor, then we would have to write another sprite cache or manager for the Editor in C#.

Editor drawing sprites

If we remake sprite cache into a C# cache/manager, then there very likely is a need to remake remaining game content drawing into C# code as well. This task may be stated in a separate ticket.

Compiling sprite file

The existing C++ code relies on sprites stored in a native C++ sprite cache object, which is going to be inconvenient.

The options here are:

Writing stand-alone tool may be picked out to a separate ticket.

selmiak commented 3 years ago

That is a great plan! About the specifics, what about calling the sprite folder "SpriteCache"? I, as a frequent AGS user, would have never called my own sprite folder this and it will hopefully not collide with existing folders. To be on the safer side about the user doing crap with the SpriteCache, what about making that a hidden folder?

"Personally I doubt if subfolders within "Sprites" should correspond to the folders in the editor's sprite manager." There are a lot of possible workflows of managing the game assets, so the workflow I suggest is to have the user have a user sprite folder somewhere where the user has his own logic and sorting and probably more source files and photoshop files and references and sketches and more things not needed in the game's sprite cache and the SpriteCache folder where the user usually does nothing with. Thus I suggest to make it a hidden folder. Maybe even add a short textfile to it called "!get_out.txt" and in that explain that the user should not tinker with the files in that folder or noone can help him. The ! at the beginning of the filename should push it to the top of the list, at least under windows. Since we can have folders in the Editor to make it easier to use, why not replicate that structure in the SpriteCache in case some user still looks in there. Is there any downside to replicating the structure of the editor in the cache folder? Non that I can think of atm.

Importing: on import the editor should just make a 1:1 copy of the sprite the user imports in the cache folder (including corresponding subfolder) and call it like the editor sprite number and according suffix. Probably with leading zeros. 00001.jpg, 00002.png etc. When the user imports a spritesheet with 8 frames from a single source image as 8 images in the editor, the editor makes that 8 images in the sprite cache folder with corresponding number filename. And with that workflow of the editor we could have a 00001t.jpg (note the t in the filename) and a 00002t.png generated which is a downscaled version of the image to display in the editor and in the views. Well, I call my thumbnails like that, maybe a dedicated thumbnails folder with files with the same name in it could also work but makes it easier for the user to screw things up by moving files and overwriting the actual images. And a user could have a "thumbnails" folder in the editor, that would make it complicated to replicate the structure.

I think the SpriteCache folder should be treated like the current single-file with all the sprites, a place where the user usually does not interfere with (thus my suggestion to make it a hidden folder)! Whatever the user does with that folder is not a problem of the editor. But in case a user is curious or tech savvy enough to browse through that folder for whatever the reasons may be to not use the editor like it is supposed to be used, why not replicate the folder structure of the editor to make it feel familiar already when viewing the sprite cache on disc? Though when the user deletes files from there, the editor has to check on startup of a project if all sprites exist in the SpriteCache and if not try to reload them from the source directory. And when even reloading from the user folder fails the editor needs to generate a 1x1px placeholder or a default "missing image" file on that number or something similar to not screw up usage of imagenumber in views and in script. The user would have to reimport that sprite number then (or live with the placeholders in game). If a thumbnail is found to be missing on startup of a project that can be generated from the sprite cache again. The thumbnails could be optional in game settings. Or they could be generated new each time on startup of a project and deleted on closing the project to save diskspace, but that could lead to very long startup times of a project. Maybe that could also be an optional setting in the game settings.

ivan-mogilko commented 3 years ago

@selmiak the original idea was to have actual source sprites inside a project's subfolder, where user can work, directly editing these sprites, and editor would catch these changes up.

You are speaking of a "sprite cache", which is a different thing, it contains sprites already converted into the runtime resource.

We must separate these two (sources and cache), and decide how to handle both. This "sprite cache" could either be an alternate approach to having sprites in the project, or it may be used alongside with the sources as an optional internal feature for the editor program's convenience or perfomance. I believe that both approaches have a potential, and their pros and cons should be investigated.

selmiak commented 3 years ago

I understand, so this is an open discussion and that is my take on it. Both ways have their benefits, but with my proposal the used files get renamed to their sprite number, having these besides your work files makes your work folders cluttered, especially for animations. If having only one folder means not renaming the files and just having a pointer, that would work too.

atm the user has no access to the files imported into the gameproject as they are in the spritefile, and the audio import works similar if I'm not mistaken, this keeps the chances of corrupting things (by the user, not by hardware failure) low. Imho this should be kept that way, maybe a bit more open with my proposal of the hidden folder. SpriteCache is just a name suggestion for that folder, after reading your post maybe EditorSpriteCache would be more fitting? but also very long. IngameSprites. GameSprites. ImportedSprites. Just sprites could indeed be used already, IngameSprites and GameSprites have a chance of already existing made by the user too. I just settled for something with cache in it, as no user will call a folder something with cache. At least not in 99.9% of all cases... For a single sprite sheet imported and cut up into 8 single frames and stored as 8 different files this is a spritecache in my definition of the word. So EditorSpriteCache seems fitting. But as long as the runtime does not store the sprites converted into runtime ressources in a SpriteCache Folder there should be no collission and it is just shorter and it is usually hidden away from the user anyways and we can just define the hidden SpriteCache folder as the editor Sprite Cache...

So much for that proposal, and I agree putting together a list of all the pros and cons of either way (or maybe getting more input here and having a 3rd way with more pros and cons) is a good idea and a good base to make a decision and a plan. I hope I'll come back to that and write up some pros and cons over the next few days...

persn commented 3 years ago

Don't get creative with the naming, it's only going to annoy us and make it difficult to understand what the folder is for. Calling it "AnythingCache" will just be misleading since it's no longer going to be a cache. Conflicts with existing files are not really a concern since we will probably just take existing files if they exist and move them into a backup folder. This is the strategy we're already attempting to implement with a "Rooms" folder in the open room format PR. Of course that is a WIP and is not final as of right now #1278

I'm not really keen on the idea of making a hidden folder here, we're trying to move to a "open" format. Part of the goal is exactly that users can see and edit the files directly from disk without having to rely on the editor. I don't safety for user screwups is a use-case we should value because I think the a lot of people will not be brave enough to mess around with the files when they don't know or understand how to do so.

ivan-mogilko commented 3 years ago

Yes, things like the folder naming and file naming are late moment issues, we should not waste time on these now. Same goes for utility improvements, such as thumbnails, as these are not part of the game - these are completely separate problems.

At first we need a general principle: what data do we keep in the project, how it is modified by the user, how it is handled by the tools (game compilation tools), how it is packed for the game.

The current sprite pipeline looks like this:

Source sprite + Import settings => Runtime sprite => Runtime sprite package

From all this only "Source sprite + Import settings" are essential for the project, because other parts may be safely deleted and restored later from the source.

One of the questions that was raised in the past was: must the "Source sprites" be considered a part of the project; if so then they should be stored inside the project folder.

Another requirement made in the past was to be able to edit and then fully compile a game (and any of its components) without launching the editor. This means that the user may edit the original sprite, then run a standalone tool (or a script that calls number of tools in a sequence), and sprites go all the way from the source form to packed for the engine. This also means that the editor itself should not define how sprites are stored in the project. Editor of course may keep any extra files for itself in the workspace or other temp location, but any additional enhancements to how editor works with sprites are considered separate issues.

selmiak commented 3 years ago

alright, I don't want to decide how that cache folder, if it ever is needed, is called. So with the open folder is in the game system, will the AGS editor just browse a selected folder on project startup and import everything it finds into the game? or will the user still have to select the files to be displayed in the sprites section of the game project? Second is probably going to work just fine, autobrwosing would be bad as my working folders for gfx tend to be way less organized and more messy than the in gameproject sprites, I have sketches and different version of images in these folders that are not needed in the game project or the finished game's ressource at all!

persn commented 3 years ago

The data for all the sprites and how they will show as folders in the editor are found inside the Game.agf file. Anything not added there will not show.

image

persn commented 2 years ago

I got started on a branch for this while I was awaiting feedback on the open room format PR a while back, but it has been collecting dust for a while now. I didn't get super far, it takes the sprites through the native poxy, dumps it to disk, and there's a sprite drawing that draws using sprite from disk that needs more work.

I might do more on it if I can, but just to be safe, I'll try to rebase it on ags4 branch and post it here this weekend so anyone else could use it for reference if I don't have time

ivan-mogilko commented 2 years ago

@persn may be relevant, recently I wrote a managed SpriteFileWriter wrapper meant to reconstruct spritefile by reimporting all sprites from sources, perhaps could be seen as a reference for spritefile compiling from the project assets: #1594

persn commented 2 years ago

@persn may be relevant, recently I wrote a managed SpriteFileWriter wrapper meant to reconstruct spritefile by reimporting all sprites from sources, perhaps could be seen as a reference for spritefile compiling from the project assets: https://github.com/adventuregamestudio/ags/pull/1594

I imagine one of the end goals will be to have the ability to gitignore the sprite file (this should be possible now with .crm files), so that would be absolutely relevant

persn commented 2 years ago

I got started on a branch for this

https://github.com/persn/ags/tree/feature/open-sprites

I'll keep it up to date if I make any more changes

ericoporto commented 1 year ago

Talked a bit with @persn, and he mentioned that his branch was showing two PRs were necessary.

ivan-mogilko commented 1 year ago

I was going to write this for a while, but could not find spare time.

I think that besides the above, there's another change that would be necessary, and unfortunately it was not mentioned in the ticket (as far as I can see). This is related to test-running the game.

The spritefile compilation may take some time, if there are alot of sprites. Right now it's being updated as you import sprites and save the project in the Editor, and the long wait time only happens when you save after adding or changing sprites. But this process copies parts of the previous file to speed things up a bit.

If we change this to have sprites on disk, and spritefile built only during game compilation, that raises the question about how to update a spritefile. Because if the spritefile is fully recreated each time by doing a import from separate files (let alone doing tiled import etc, - I am not fully certain how optimized the storage would be), then this process will become slower than before.

Therefore, the Editor would either have to keep a record of what has changed since the last spritefile recompilation, track the time of modification of a spritefile, etc. Or there will have to be some kind of an intermediate form of sprite storage, between the source image files in the folder, and compiled spritefile, that would speed up gathering final game file.

In the second case, for the optimal workflow, it would be desired to also let Engine use that intermediate form, in such case the full recompilation would not be necessary during the test run.

There is this ticket #702, which somehow got almost 50 comments in the discussion, and for the most part we discussed various options of solving similar problem for a different reason. If I recall right, the discussed methods may be summarized into following variants:

  1. Store the results of import and conversion to raw bitmap (or whatever engine can load) in separate files (1 file per sprite). That is - not true BMP files with BM header and such, but raw chunks of pixel data in the same format as these chunks are written in the spritefile (may even be compressed).
  2. Have a "dirty" spritefile which stores both old "deleted" sprites and new ones and replacements to old ones. Don't delete old sprite data, only append. Use index file to know where are the true sprites in the main file. Every now and then recompile the spritefile to remove deprecated chunks.
  3. An extended variant of p2: store a dirty spritefile in multiple partitions. When you change something, only append to the respective partition, or start a new one.

The idea here is that besides the editor, that should handle this intermediate storage, the Engine should also be taught to read from it. Since now we have a completely separate SpriteFile class that streams from a file, and SpriteCache itself does not care how the sprite data is stored on disk, it will be possible to have different implementations of the same SpriteFile interface, including one reading from separate files, etc. They likely will work slower than streaming from 1 file, but with a well-sized cache that may not be a big problem. The particular implementation may be allocated according to a game setting or a command line arg.

Above is a problem overview, I think we would need a dedicated ticket for this. Note that likely this may be worked on in parallel to the other subtasks.

ericoporto commented 1 year ago

Because we can recreate the spritefile from source files nowadays, I still don't quite grasp what this open format adds :/. Additionally, as sprites are sequential numbered items, a merge of branches will always entail manual sprite index adjustment or following practices from cisbetter tutorial.

ivan-mogilko commented 1 year ago

Because we can recreate the spritefile from source files nowadays, I still don't quite grasp what this open format adds

Our goal is to have fully open project source, where everything is stored in a human-readable format, without using "magic packages", where:

Additionally, although maybe less obvious:

The sprites are the last part remaining in which editor is relying on the presence of a compiled package, practically using an output format to work. On another hand, the "sprite sources" are already a part of a workflow, but they are practically not considered a part of the project: they do not have to be inside a project folder, and do not need to be even present for project to work. If you change the "sprite source", the project is not considered updated until user does some manual action in the editor. We want to make Editor not rely on the output format, and rely on these "sprite sources" instead. We want their presence to be required for the project to be considered "valid", and the state of these source files to be part of the project's state, so to speak.

Additionally, as sprites are sequential numbered items, a merge of branches will always entail manual sprite index adjustment

Yes, this problem still persist, and have to be addressed separately. I might suggest discussing this on forums, to see if people have any interesting ideas.

One idea that I once had is to implement a workspace setting that defines a range of sprite IDs to use when importing new sprites. Then each team member could define different range for themselves, and thus avoid conflicts. Or configure sprite folders for the same purposes.

If the sprite storage format changes, maybe that may also bring possible solutions, like, adding sprite id in filenames, or something like that.

persn commented 1 year ago

Talked a bit with @persn, and he mentioned that his branch was showing two PRs were necessary.

  • 1st PR: Consolidate sprite code using the ISpriteController interface, similar to IRoomController for open room format. Today, multiple rendering methods exist, including an unrelated custom drawing routine in the editor plugin interface, these have to be unified in a way to avoid code duplication.
  • 2nd PR: Replace the interface implementation with code that reads from the open format.

Would PR 1 be suitable for the master branch? Or the ags4 branch?

ivan-mogilko commented 1 year ago

Would PR 1 be suitable for the master branch? Or the ags4 branch?

Could you elaborate on which changes will this include? Overall 3.* branch is today meant primarily for improving existing features (performance, convenience, cleaning the code), larger changes should go into ags4.

ericoporto commented 1 year ago

I believe it fits with cleaning the code, so perhaps it can be in master and then later merged to ags4.

persn commented 1 year ago

Could you elaborate on which changes will this include? Overall 3.* branch is today meant primarily for improving existing features (performance, convenience, cleaning the code), larger changes should go into ags4.

I don't think I can explain it anymore plainly than what's already written. It's probably easy enough to rebase, so it would make sense to just base the branch off master, and then rebase on request after a PR review if the changes got out of scope