Issue 1 add new animations

[jrconway3]

I kept trying to fix the export but it wasn't working correctly, though now thinking it over, maybe its actually lines 553-556 I needed to update to make "sit" work with the existing structure of sit. Originally I set up emote as a similar structure, but because that was a new sheet, I didn't think it mattered much to drop that format.

When I left it at "from_rows" I could not get "sit" to split the spritesheets out. I thought the fix needed to be done in the layout/universal-expanded.json, but thinking it over, it might have needed to actually be in arrange.py lines 553-556. If that's the case let me know, I can fix that.

Anyway, here's the commands I ran to generate assets for all frames on the ULPC now:

lpctools arrange distribute --layout universal-expanded --input path/to/split/assets --output path/to/asset/universal.png --offsets path/to/reference_points_expanded.png --mask path/to/masks_male_expanded.png
lpctools arrange separate --layout universal-expanded --input path/to/asset/universal.png --output-dir path/to/asset/_separated
lpctools colors recolor --input path/to/asset/_separated/*.png --mapping path/to/lettes/palette.json

Only notable issue is that "separate" turns "spellcast" into "cast". In the ULPC we use "spellcast" now. I thought about changing this in here, but didn't want to change things too much.

No grab/carry yet, but I want to do those soon-ish. (I might do a different grab because I doubt we'll get OGA-BY support from Daneeklu, I can probably use hand frames from another asset to make this line up and still look good, while keeping the basic structure here). I also want to make a "lift" animation, too.

Otherwise, I'll probably use the existing format for the most part and just keep a simple grab/carry that only changes the arms while keeping the legs the same.

EDIT:
Nvm, I tried setting up the sit section in lines 553-555 to work with this format and it didn't work, so I have no idea how to make it work with the from_rows format.

[bluecarrot16]

Hi, thanks for working on this!

1. Your usage commands seem right.

2. Let me clarify that the files in tests/*_files/ are only necessary for running the test suite (and to serve as examples). The file tests/arrange_files/layout/universal-expanded.json is not read by the library unless you specifically reference it with a command line option; for example, you could run:

lpctools arrange distribute --layout tests/arrange_files/layout/universal-expanded.json --input path/to/split/assets --output path/to/asset/universal.png --offsets path/to/reference_points_expanded.png --mask path/to/masks_male_expanded.png

Adding the file tests/arrange_files/layout/universal-expanded.json does not allow the library to understand --layout universal-expanded. The place to edit "built-in" layouts is in arrange.py, as you seem to have correctly done.

3. There appear to be a number of tests failing (run the tests with pytest from the main directory). Some of those are because you've updated files without updating the associated test (e.g. arrange_files/hair/hair_page2.png which is referenced in test_distribute_hair), and some of them are probably because of Tests fail because new Pillow version does not save exif metadata (by default) whereas test case images have it #4 . I'll address the latter; could you try to address the former (make a new test using the new image and/or update the layout in the test cases to use your new 'universal-expanded' layout?

4. I don't understand the issue with sit; can you give an example of what is happening that you don't expect (or better yet, write a failing test)?

5. Regarding cast/spellcast, the separate verb currently makes one file per animation, using the animation names configured in the layout (e.g. in layouts = {... from arrange.py. We could easily add a CLI argument like --rename-animation FROM=TO to rename "cast" to "spellcast" for the sake of generator. Or we could change it to "spellcast" throughout in this library, as long as we change the tests/test cases too. Either (or both) is fine, but could you open a separate issue for this?

[jrconway3]

Yeah I made a mistake with updating those hair files. I was just testing the expanded layout command.

Anyway, I'll go set up the original "sit" error I was getting and drop that in. And yeah, I can create a new issue for spellcast.

I did run into a MAJOR issue with spellcast recently. I separated a lot of assets at once (33 facial expressions) and forgot to rename cast to spellcast, so I had some complications in regenerating the list. I ended up renaming the main cast.png file to spellcast.png and then regenerating the colors that way, but it was a pain because I had dozens of files/directories like that. Thankfully I figured out how to use mmv which solved the problem.

[jrconway3]

@bluecarrot16 So here's the error I'm getting with sit:

lpctools arrange separate --layout universal-expanded --input characters/updates/head/faces/global/tears.png --output-dir characters/updates/head/faces/global/tears/
Warning: missing AnimationFrameID(name='sit-ground', direction='n', frame=0) for this layout
Warning: missing AnimationFrameID(name='sit-cross', direction='n', frame=1) for this layout
Warning: missing AnimationFrameID(name='sit-chair', direction='n', frame=2) for this layout

This goes for all directions and I get a huge wall of this. I'm only showing the basics.

[bluecarrot16]

Hmm, are you sure you don't have uncommitted changes? Looks like it's working as I expect...

When I checkout this branch and run:

mkdir tmp_hair
lpctools arrange separate --layout universal-expanded --input tests/arrange_files/hair/hair_page2.png --output-dir tmp_hair

lpctools exits without error (or output), and the appropriate output files appear to be generated:

ls -1 tmp_hair
backslash.png
cast.png
climb.png
combat_idle.png
emote.png
halfslash.png
hurt.png
idle.png
jump.png
run.png
shoot.png
sit.png
slash.png
thrust.png
walk.png

Here's what sit.png looks like:

Comparing to the current version in the generator, seems like this is the desired behavior?

---

I see that in the past you had defined separate animations for "sit-ground", "sit-cross" and "sit-chair"; this would be fine too, it would just need to be used consistently. The hangup is perhaps that the separate verb creates one image per animation, so it would try to create images sit-ground.png, sit-cross.png, etc. With that definition of the layout, to produce behavior consistent with the current generator (i.e. producing sit.png), we would need to use the repack verb instead of separate, something like this:

lpctools arrange separate --from universal-expanded --to sit cast thrust ...etc... --input tests/arrange_files/hair/hair_page2.png --output-dir tmp_hair 

Does that make sense?

Anyway the current situation seems simpler to me. Someone can always define layouts "sit-ground", "sit-cross", etc. like this:

'sit-ground': SpritesheetLayout.from_rows([
		[ ('sit'        , 'n' , 0) ] ,
		[ ('sit'        , 'w' , 0) ] ,
		[ ('sit'        , 's' , 0) ] ,
		[ ('sit'        , 'e' , 0) ] ,
]),
'sit-cross': SpritesheetLayout.from_rows([
		[ ('sit'        , 'n' , 1) ] ,
		[ ('sit'        , 'w' , 1) ] ,
		[ ('sit'        , 's' , 1) ] ,
		[ ('sit'        , 'e' , 1) ] ,
]),

or in sit-ground.json:

{
	"frame_size": [64, 64], 
	"size": [1, 4], 
	"rows": [
		[{"name": "sit", "direction": "n", "frame": 0}],
		[{"name": "sit", "direction": "w", "frame": 0}],
		[{"name": "sit", "direction": "s", "frame": 0}],
		[{"name": "sit", "direction": "e", "frame": 0}],
	]
}

[jrconway3]

Well, if you check the source code:

	#'sit': SpritesheetLayout.from_rows([
	#		[('sit-ground' , 'n' , 0), ('sit-cross' , 'n' , 1), ('sit-chair' , 'n' , 2)],
	#		[('sit-ground' , 'w' , 0), ('sit-cross' , 'w' , 1), ('sit-chair' , 'w' , 2)],
	#		[('sit-ground' , 's' , 0), ('sit-cross' , 's' , 1), ('sit-chair' , 's' , 2)],
	#		[('sit-ground' , 'e' , 0), ('sit-cross' , 'e' , 1), ('sit-chair' , 'e' , 2)],
	#	]),
	'sit': SpritesheetLayout.from_animation('sit', 3),

I was simply saying that I had to comment out the format you had already and set it up this way in order to fix it. If you're fine with this, then we can keep this as is (or even remove the comments). My comment on the sit was that the reason why I commented it out was because I couldn't get around that error.

Even when I tried to use sit-ground, sit-cross, sit-chair in the layout, I still couldn't get it to work, so I went to comment out the old method and switched to use from_animation instead.

[bluecarrot16]

Hmm, it looks like the way you defined the universal-expanded layout in that version used animations called "sit"

lpctools/lpctools/arrange.py

Lines 557 to 560 in a0da01f

	[ ('sit' , 'n' , range(3)) ] ,
	[ ('sit' , 'w' , range(3)) ] ,
	[ ('sit' , 's' , range(3)) ] ,
	[ ('sit' , 'e' , range(3)) ] ,

but then I had a layout called "sit" which referred to separately-named animations "sit-ground", "sit-cross", and "sit-chair":

lpctools/lpctools/arrange.py

Lines 668 to 673 in a0da01f

	'sit': SpritesheetLayout.from_rows([
	[('sit-ground' , 'n' , 0), ('sit-cross' , 'n' , 0), ('sit-chair' , 'n' , 0)],
	[('sit-ground' , 'w' , 0), ('sit-cross' , 'w' , 0), ('sit-chair' , 'w' , 0)],
	[('sit-ground' , 's' , 0), ('sit-cross' , 's' , 0), ('sit-chair' , 's' , 0)],
	[('sit-ground' , 'e' , 0), ('sit-cross' , 'e' , 0), ('sit-chair' , 'e' , 0)],
	]),

(I forgot that I had added that long ago...)

Internally, if you look at the code for separate, it reads the animation name (i.e. "sit") and tries to repack to a layout with the same name ("sit"). In your original draft, the code was loading a bunch of frames named 'sit' from the 'universal-expandedlayout, but then trying to write frames named'sit-ground', 'sit-cross', etc. based on the definition of the 'sit'` layout. It couldn't find them, which is what the error is saying. That's what I mean that the different names could be used, but it would need to be consistent.

universal-expanded would need to be defined like this:

'universal-expanded': SpritesheetLayout.from_rows([
	[ ('cast'       , 'n' , range(7)) ] ,
	[ ('cast'       , 'w' , range(7)) ] ,
	[ ('cast'       , 's' , range(7)) ] ,
	[ ('cast'       , 'e' , range(7)) ] ,
	# ... snip ...
	[ ('jump'       , 'n' , range(5)) ] ,
	[ ('jump'       , 'w' , range(5)) ] ,
	[ ('jump'       , 's' , range(5)) ] ,
	[ ('jump'       , 'e' , range(5)) ] ,
	[ ('sit-ground' , 'n' , 0), ('sit-cross' , 'n' , 0), ('sit-chair' , 'n' , 0)],
	[ ('sit-ground' , 'w' , 0), ('sit-cross' , 'w' , 0), ('sit-chair' , 'w' , 0)],
	[ ('sit-ground' , 's' , 0), ('sit-cross' , 's' , 0), ('sit-chair' , 's' , 0)],
	[ ('sit-ground' , 'e' , 0), ('sit-cross' , 'e' , 0), ('sit-chair' , 'e' , 0)],
	[ ('emote'     , 'n' , range(3)) ] ,
	[ ('emote'     , 'w' , range(3)) ] ,
	[ ('emote'     , 's' , range(3)) ] ,
	[ ('emote'     , 'e' , range(3)) ] ,
	# ... snip ...
	[ ('halfslash'  , 'n' , range(6)) ] ,
	[ ('halfslash'  , 'w' , range(6)) ] ,
	[ ('halfslash'  , 's' , range(6)) ] ,
	[ ('halfslash'  , 'e' , range(6)) ]
]),

But anyway, yeah I suppose I prefer the current behavior...? Unless you can think of a good reason why they should be separate...

[jrconway3]

	[ ('sit-ground' , 'n' , 0), ('sit-cross' , 'n' , 0), ('sit-chair' , 'n' , 0)],
	[ ('sit-ground' , 'w' , 0), ('sit-cross' , 'w' , 0), ('sit-chair' , 'w' , 0)],
	[ ('sit-ground' , 's' , 0), ('sit-cross' , 's' , 0), ('sit-chair' , 's' , 0)],
	[ ('sit-ground' , 'e' , 0), ('sit-cross' , 'e' , 0), ('sit-chair' , 'e' , 0)],

I did try this at one point, but it didn't work, so I gave up on it. The current method works fine and I see no reason to go out of our way to make this separate. Far as I am concerned, it makes sense to just call it "sit" with 3 frames.

I'll go ahead and revert the three hair images I converted and make sure the pytest works. MAYBE I'll implement a new test for the new layout as well, though I'm not quite sure how to do that just yet...

[jrconway3]

I'll resume working on this in the next couple of days. Sorry for the delay.

[jrconway3]

@bluecarrot16 Okay, please check this over again.

I added a couple of tests with my new expanded view and made sure the only existing broken tests are ones that are ALREADY broken. This is test_unpack and test_convert_layout.

From what I'm seeing, the issues here are that for some reason, the grab files do not match for test_unpack, and the universal PNG is for some reason not matching. I thought about trying to regenerate the files, but I assume this is just an issue with pillow.

The new tests I created are just clones of your existing ones but with the expanded sheet instead.