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

FlxAsepriteUtils demo #326

Merged
merged 6 commits into from
Oct 25, 2023
Merged

Conversation

MondayHopscotch
Copy link
Contributor

No description provided.

Copy link
Member

@Geokureli Geokureli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flash is lame, gotta work around these "Cannot skip non-nullable argument X" errors in all official Flixel code

Other/FlxAspriteUtils/source/PlayState.hx Outdated Show resolved Hide resolved
Other/FlxAspriteUtils/source/PlayState.hx Outdated Show resolved Hide resolved
Other/FlxAspriteUtils/source/PlayState.hx Outdated Show resolved Hide resolved
@MondayHopscotch
Copy link
Contributor Author

Been on vacation the last week. Just getting back to cleaning this up more. I've fixed the flash target errors.

I also expanded it to swap between loading tags by index and loading tags by prefix. I'm trying to think of a way to show of the difference between these. At the moment, they result in the same exact sprite and animations. Thoughts on a good way to demonstrate that?

@Geokureli
Copy link
Member

it might not be necessary to "show" this in the demo, maybe just choose one and write a comment above/below it

FlxAsepriteUtil.loadAseAtlasAndTagsByIndex(player, "assets/player.png", "assets/player.json");
// can also use FlxAsepriteUtil.loadAseAtlasAndTagsByPrefix(player, "assets/player.png", "assets/player.json");

@MondayHopscotch
Copy link
Contributor Author

I was hoping to capture a basic use case for each method to give folks a possible reason to go one way or the other. I think I'm also trying to figure out why I personally would use the prefix. I know we chatted about this kind of stuff when we were tinkering with all of this the first time around. Exporting the sprite sheet as hash vs array is the only thing I can think of right now (as "by index" doesn't participate well with hash export).

So the trade-off (and main difference, at least that I can come up with at the moment) is that loading by index doesn't work with hash export from aseprite, and loading by prefix doesn't work unless you provide a proper export 'item filename' (which imo is terrible name for what to call each frame).

@MondayHopscotch
Copy link
Contributor Author

^ Adding on to previous comment, I just installed Aseprite on a fresh laptop that's never had it installed and the default Item Filename is {title} {frame}.{extension}, which means loading by prefix won't work with default settings. The funny part is that it also defaults to exporting the json as Hash, which means loading by index ALSO doesn't work with default settings. 😆

@Geokureli Geokureli dismissed their stale review October 17, 2023 21:23

flash errors fixed, but I still haven't tested any of this yet

@Geokureli
Copy link
Member

I think the best option is to make a flixel plugin for Aseprite that simplifies the export process. I'm not sure how it would do that or how to make this plugin, so for now we have verbose comments on these util functions and, hopefully, a demo with an included .aseprite file with the export options saved

@MondayHopscotch
Copy link
Contributor Author

If we go the route of having a flixel plugin (which I'm assuming a script is what you had in mind), where would we publish that / direct users to it? I'm going to, as part of this PR, see if I can make a preliminary script that acts as a Flixel Atlas Export tool that uses some nice default settings for exporting a spritesheet/atlas. I'll probably reach out on the discord if/when I come up with something that seems usable as it certainly won't be part of this PR.

For the sake of this PR at the moment you mentioned "export options saved." It does appear that the export options are saved within the .ase file based on some preliminary testing. That should mean that the player.ase file included in this PR does have reasonable settings for the export already.

@MondayHopscotch
Copy link
Contributor Author

MondayHopscotch commented Oct 18, 2023

@Geokureli -- So I took the liberty of playing around with Aseprite plugins/scripting. Came up with this:

Aseprite_t5QVfCN2a3.mp4

Summary of what's happening in the video:

  1. Do a standard export and set "bad" settings for flixel
  2. Use new Plugin and show that "good" settings are now configured in the export dialog

Doing a regular "Export Sprite Sheet" will just use whatever settings you last used. It defaults to Hash and some fairly unhelpful Item Filename. My plugin adds a Flixel Atlas Export menu option that lets the user provide a path for the image and json files. Clicking Confirm passes those chosen paths, along with "good" export settings to the regular Export Sprite Sheet dialog so the user can export something that works well with the flixel utilities.

Code is here:
https://github.com/MondayHopscotch/aseprite-scripts/tree/master/flixel-atlas-export

I can also make the export happen automatically without showing the second dialog, but left it in for now to show what is happening. Not sure how we'd distribute something like this, but this seems like the start of something that aligns with what you were talking about.

Let me know your thoughts!

@Geokureli
Copy link
Member

Geokureli commented Oct 18, 2023

This is amazing, I can't believe you did this so fast! for distribution, I noticed that Aseprite isn't even on this list yet, so I can add it there and host a download link, what kind of file is the plugin?

@MondayHopscotch
Copy link
Contributor Author

I was away from all technology for a week straight, so I'm quite refreshed and eager to be productive 😄

The plugins are just scripts with some extra data on them. The file itself is just a zip, but with a .aseprite-extension file extension for OS file association purposes. More info here in their docs

I've put together a little more on the repo, so the first draft of download/instructions is here.

I've also expanded the dialog a bit to hopefully make it more friendly/usable. Happy to keep expanding/tweaking this if you have requests/concerns/whatever. First time digging into all this Aseprite Scripting/Plugins/Lua stuff, so I'm quite green on all of this myself.

@Geokureli
Copy link
Member

Created a separate issue for the plugin

@MondayHopscotch MondayHopscotch marked this pull request as ready for review October 20, 2023 16:27
@Geokureli
Copy link
Member

Thoughts on folders this should be in besides "Other"? I realize this folder contains FlxAtlas and FrameCollection; perhaps we should make a new folder called "Animation", "Atlases" or "Graphics". Alternatively this can go in the "Editors" folder

@MondayHopscotch
Copy link
Contributor Author

I like 'graphics' as that more closely follows the flixel code structure. Maybe that makes it more intuitive to navigate the demos?

@Geokureli
Copy link
Member

did you test this with the latest release, or dev branch? I tried it with both and the animations aren't animating, they either play the first frame of the animation or stop on the second frame of the animation.

in 5.4.1 we read the "repeat" value to determine whether an animation loops, I think you might be using 5.4.0
Screenshot 2023-10-23 at 10 05 23 PM

Also it might be a good idea to change the duration of some of the frames, maybe like 250

@MondayHopscotch
Copy link
Contributor Author

Oops, I had pull the latest from flixel-demos, but forgot to set my haxelib to use flixel dev branch. I'll update and do a round of testing with that.

I'm also noticing UI behavior that I didn't notice before (more Aseprite odd choices). In the tag properties dialog, if the Repeat: box isn't checked, the field is grayed out and shows . So not having repeat checked appears to imply looping forever. Thoughts on that? It feels almost like this line should be return this == null || toInt() > 1; or something. I know flixel animations don't have the notion of "how many times to loop", but something I noticed while working on this demo.

@Geokureli
Copy link
Member

I'm also noticing UI behavior that I didn't notice before (more Aseprite odd choices). In the tag properties dialog, if the Repeat: box isn't checked, the field is grayed out and shows . So not having repeat checked appears to imply looping forever. Thoughts on that?

I bring this up here: aseprite/aseprite#1989 (comment) i find it very unintuitive

It feels almost like this line should be return this == null || toInt() > 1; or something. I know flixel animations don't have the notion of "how many times to loop", but something I noticed while working on this demo.

To me this just adds more unintuitive steps and friction to an already unintuitive system, im wondering if the plugin can solve this

@MondayHopscotch
Copy link
Contributor Author

I agree that the naming is a bit weird, specifically that the unchecked "Repeat" checkbox means it repeats literally as much as possible (forever) 😄

To me this just adds more unintuitive steps and friction to an already unintuitive system, im wondering if the plugin can solve this

Can you elaborate on what you mean by "more unintuitive steps"?

But the more I think on this, the more I think that using return this == null || toInt() > 1; to set looping feels more intuitive with how Aseprite's workflow works. All animations in Aseprite default to having the Repeat box unchecked and loop forever in the editor. So the intuitive thing to do is to have flixel mirror the editor's behavior in the default case - which would be the this == null part of that expression.

@Geokureli
Copy link
Member

I keep forgetting that "Repeat: 1" means "Never Repeat" and 2 means "Repeat Once" this field is just so stupid, I see where you get > 1, now

have flixel mirror the editor's behavior
this makes sense, except for the fact that the editor's behavior makes no sense.

let's have the demo use repeat:2 and start an issue in flixel about return this == null || toInt() > 1

@MondayHopscotch
Copy link
Contributor Author

I have created an issue and corresponding PR (linked in issue): HaxeFlixel/flixel#2947

@Geokureli
Copy link
Member

all good, waiting on CI

@Geokureli Geokureli merged commit 0ef57f0 into HaxeFlixel:dev Oct 25, 2023
15 checks passed
@MondayHopscotch MondayHopscotch deleted the flxasepriteutil-demo branch October 25, 2023 06:47
Geokureli added a commit that referenced this pull request Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants