refactor: streamline grid coords -> pixel coords

[andretchen0]

This is a cleanup of #2443, addressing issues raised there, except for sprite texture "feet", e.g. Uncle Fungus positioning.

As positioning those elements might require a fair amount of tweaking, I'll submit it as a separate PR.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
ancientbeast	✅ Ready (Inspect)	Visit Preview	Jul 20, 2023 6:19pm

[ghost]

👇 Click on the image for a new way to code review

Legend

[DreadKnight]

Heya! In case you missed my comment in the chaos created: #2443 (comment)

So please no more PR closing for sake of clean history. Also, the anchors should be restored, they are there for a reason. Nobody will bother rendering stuff in silly ways when we'll be rendering 3d units and same goes for having templates for 2d stuff. Leaving blank spaces in sprites is a big NO regarding flow and optimization, nobody sane does that stuff overall.
I guess you understood that stuff, but the refactor probably needs extra manual work to align stuff nicely, just making sure that we're on the same page now. Hopefully unit placement will be matching the current beta version of the game 🐻

[DreadKnight]

Did some more testing. So besides unit placement:

• hex grid coordinates (shift) need to be centered
• traps need to get squished as well, like hexagons
• Snow Bunny's Chilling Spit vanishes prematurely
• Impaler's Poisonous Vine should be like in beta
  (sort of vertical on the ground itself, not squished)

[andretchen0]

This PR has been a rougher ride than the other ones.

In part, that's my fault. I closed the earlier PR and then had troubles with subsequent PRs for some reason – one ballooned to ~30 MB.

In part, it's just the nature of the changes – it touches almost everything rendered in Phaser. But I think the changes are worthwhile.

Also, the anchors should be restored, they are there for a reason.

I'm not sure which ones you're referring to. Are you talking about the offsets that are based on display data from units.json?

Nobody will bother rendering stuff in silly ways when we'll be rendering 3d units and same goes for having templates for 2d stuff. Leaving blank spaces in sprites is a big NO regarding flow and optimization, nobody sane does that stuff overall.

If you're open to discussion, here's my point of view:

If by optimization, you mean that there shouldn't be empty space in the sprite sheet, for a game of this size, it mostly doesn't matter.

But about "Nobody will bother rendering stuff in silly ways", here's a sprite sheet from a winning Ludum Dare game.

Notice all the red bearded character when he's punching. He's punching right and there are a bunch of empty pixels to the left. That puts the character in middle of the sprite frame. It can flipped in-game without adjustment. There are no offsets to deal with in code. Here's the sprite atlas – notice all the offsets are 0, 0. To my knowledge, he doesn't pick that data up in-game.

That's from the guy behind Dead Cells who's also a serial Ludum Dare winner, fwiw. Not just a random game jam participant.

That's the same thing I'm doing here.

It's my view anyway that code complexity is slowing this project down and if I find existing code that can be replaced with no code at all, then that's a win.

I guess you understood that stuff, but the refactor probably needs extra manual work to align stuff nicely, just making sure that we're on the same page now.

Before getting your feedback, I modified the sprites to align with the way they appear on-screen in the beta version. I also removed the display data from units.json. That's on my local and ready to go. Do you want me to submit that? I don't want it to be perceived as me thumbing my nose at you.

[DreadKnight]

This PR has been a rougher ride than the other ones.

In part, that's my fault. I closed the earlier PR and then had troubles with subsequent PRs for some reason – one ballooned to ~30 MB.

Fun. Well, perhaps because graphical assets were involved as well, they tend to add up at times.

In part, it's just the nature of the changes – it touches almost everything rendered in Phaser. But I think the changes are worthwhile.

Also, the anchors should be restored, they are there for a reason.

I'm not sure which ones you're referring to. Are you talking about the offsets that are based on display data from units.json?

I guess so, the x,y coordinates used to move the sprite around, cardboard offsetting coordinates basically.

Nobody will bother rendering stuff in silly ways when we'll be rendering 3d units and same goes for having templates for 2d stuff. Leaving blank spaces in sprites is a big NO regarding flow and optimization, nobody sane does that stuff overall.

If you're open to discussion, here's my point of view:

I'm always open for discussion, but reading your comment full first, I can see your confusion, as expected. Always good to clarify things in order to learn new stuff or be more sure about it. I tend to learn quite a bit from you when you take the time to carefully go over things with me, always awesome to have a mentor like that :)

If by optimization, you mean that there shouldn't be empty space in the sprite sheet, for a game of this size, it mostly doesn't matter.

The empty space in sheets is for a different purpose, not to center the character. Anchors are for that sort of thing.

But about "Nobody will bother rendering stuff in silly ways", here's a sprite sheet from a winning Ludum Dare game.

Notice all the red bearded character when he's punching. He's punching right and there are a bunch of empty pixels to the left. That puts the character in middle of the sprite frame. It can flipped in-game without adjustment. There are no offsets to deal with in code. Here's the sprite atlas – notice all the offsets are 0, 0. To my knowledge, he doesn't pick that data up in-game.

Here's the thing: the empty space is only there in order to make all the frames from an action easily align without any headache at all. Character won't be flippable durring animation most likely. The character can be anywhere within that defined canvas, even in the corners, you should think of that as a video. After the animation is done, character will return to an idle type of animation, which will use another set with totally different frame sizes most likely. Those 2 different action sets will be aligned together using an anchor point, so that things will seem seamless; having starting or ending frames being almost similar in a sprite sheet and between the various sets helps with the magic big time.

That's from the guy behind Dead Cells who's also a serial Ludum Dare winner, fwiw. Not just a random game jam participant.

That's the same thing I'm doing here.

Most pixel art games have very few frames, they align easily most of the time. Also, they're 2d, like sidescrollers in this case, while AB is 2.5, meaning character will be visible in a different perspective, having feet going through the floor, very visible in the case of Uncle Fungus. In the pixel art game you referenced, bottom pixel line is where character will have feet sitting on some tile, way simpler concept, as most of the time things will align there by default, but that's not always the case. On NES, there was a Teenate Mutant Ninja Turtles game and you would have Donatello with his stick hitting bellow him big time while crouching. In modern times that attack sheet would get a different anchor point.

Example from another TMNT, different actions on each row, different sprite heights and different gaps, but the thing is that this is just a rip, most are just compiled in order to expose the various frames, they're not optimized for animation, so the proper gaps are missing here, making aligning things a total nightmare, having only some borders so that frames don't touch each other:

Rows with extended bo stick should have a lot of frames contain big gaps there. So there's a bit of work to process the rip and make it into proper sprite sheets and such.

Imagine this scenario:
Abolished has some idle animation, barely moving. The sprites will be all in a single sheet, without much space in between.
Then Abolished uses Fiery Claw and stretches arm quite a bit. Sprites in that sheet will end up with a lot of blank space, before and after the hit itself, so that way it's easy to define a sprite size and get the animation going. Character won't be centered at all in the frames, the more the arm will be stretched, the more visible that will be.
So how do you go from a state to another while keeping things seamlessly? You define an anchor, think of it like a pin, aligning all those animations composed of different sprite sheets together. We might not need them that much now, but it will be obvious once we'll have animations around.
There could be no gaps at all, but gaps have their purpose, just not what you seem to think it is for our scenario as I see it. If in a spritesheet of a punching animation you don't leave any gaps, you would have to constantly move every frame into proper position (define a json with coordinates), which would be a total nightmare.

It's my view anyway that code complexity is slowing this project down and if I find existing code that can be replaced with no code at all, then that's a win.

I agree, but when it comes to this stuff, it makes no sense or we're talking about very different things and not being on the same page.

I guess you understood that stuff, but the refactor probably needs extra manual work to align stuff nicely, just making sure that we're on the same page now.

Before getting your feedback, I modified the sprites to align with the way they appear on-screen in the beta version. I also removed the display data from units.json. That's on my local and ready to go. Do you want me to submit that? I don't want it to be perceived as me thumbing my nose at you.

We need JSON data with anchors (offsets), otherwise we would need to have every single sprite sheet have the same gaps, so if some "long limb" attack is added or removed, all the sprite sheets would have to be readjusted constantly. That won't be efficient and won't make things modular at all, all because some x,y offset coordinate points. You be the judge.

[andretchen0]

You be the judge.

You seem to be passionate about the offsets/anchors and that's fine. I'll put them back in.

The offset data probably should be removed from units.json though. The other info there feeds into business rules. The offsets are view data that will (hopefully) eventually be generated by a program or script.

Also, fwiw, I don't know of a free animation program that generates offsets and that works on all platforms.

[andretchen0]

I'm guessing the offset data in units.json was added by hand. Is that right?

I can't quite make sense of the logic behind the numbers. To me, it seems to make the most sense to have the offsets define a "root anchor" – i.e., the x, y coords that represent the midline of the character on the "ground". 0, 0 is top, left.

For "bounty hunter", that'd be here (green square):

That's roughy x: 53, y: 191.

The display info from units.json is:

display: { width: 108, height: 200, 'offset-x': 0, 'offset-y': -164, },

Assuming the starting point is the top, left (0, 0) and offset-y is flipped, that puts the anchor here:

... so that's obviously not the scheme.

My guess is this is just "what worked", which is fine. But I'd like to refactor "what worked" into something that can be documented and understood down the line. The "root anchor" as described above makes sense to me anyway.

@DreadKnight: Are you ok with that?

[andretchen0]

This is an important step, but given that it touches on a lot of elements, I think this is too large a leap for the moment. I'll try to tackle it more incrementally.

[DreadKnight]

You be the judge.

You seem to be passionate about the offsets/anchors and that's fine. I'll put them back in.

I'm not passionate about them, it's how things get done. You compared this project with some game jam one 🐻

The offset data probably should be removed from units.json though. The other info there feeds into business rules. The offsets are view data that will (hopefully) eventually be generated by a program or script.

The offset data should be in a separate file eventually where corresponding animation data will be as well.

Also, fwiw, I don't know of a free animation program that generates offsets and that works on all platforms.

Can't think of something either atm, we could eventually have some more visual tool to set up the anchor/offset point.
As we have an add-on to render for blender and a sprite sheet viewer -> https://AncientBeast.com/viewer which we could eventually fork it for AB, have a settings to display 1-3 hexagons under unit and such.

[DreadKnight]

I'm guessing the offset data in units.json was added by hand. Is that right?

Yes, by hand; trial and error. Not that efficient, but it got the job done pretty well.

I can't quite make sense of the logic behind the numbers. To me, it seems to make the most sense to have the offsets define a "root anchor" – i.e., the x, y coords that represent the midline of the character on the "ground". 0, 0 is top, left.

For "bounty hunter", that'd be here (green square):

That's roughy x: 53, y: 191.

The display info from units.json is:

display: { width: 108, height: 200, 'offset-x': 0, 'offset-y': -164, },

Assuming the starting point is the top, left (0, 0) and offset-y is flipped, that puts the anchor here:

... so that's obviously not the scheme.

The "bounty hunter" example is the worst one possible. That unit is wip and without cardboard, just a temporary clone.
It was also implemented by someone else new to the project, I also haven't bothered to check it if was done properly.

My guess is this is just "what worked", which is fine. But I'd like to refactor "what worked" into something that can be documented and understood down the line. The "root anchor" as described above makes sense to me anyway.

@DreadKnight: Are you ok with that?

Yes, what works. Idea is to nicely fit the unit without 1-3 hexagons without "spilling out" too much or looking awkward.
Also, we don't have to worry about good position regarding cardboard flipping because that's kinda pointless thing btw.

[DreadKnight]

This is an important step, but given that it touches on a lot of elements, I think this is too large a leap for the moment. I'll try to tackle it more incrementally.

Yeah, modular/incremental approach is always ideal from my experience.

[andretchen0]

I'm not passionate about them, it's how things get done. You compared this project with some game jam one 🐻

I don't know what the bear emoji means here. If I've offended you:

I didn't compare AB to a game jam game – though if I had, comparing it to a game jam game from DeepKnight would be a compliment in my eyes. I linked to the game jam game because the source is online and I wanted to show you a system that works without needing to write down extra anchor data.

You want explicit anchors and that's fine.

Also, we don't have to worry about good position regarding cardboard flipping because that's kinda pointless thing btw.

I don't want to work on changes that will be unwelcome. If you'd prefer I work on something else, then go ahead and tell me what that should be.

From my point of view, above all, the codebase needs refactoring, so that common operations are easy and understandable. Drawing sprites to the screen on a particular hex and making them face a particular direction is a common operation that should be easy.

Here's the code for making a creature face one direction or another:

	faceHex(
		faceto: Hex | Creature,
		facefrom?: Hex | Creature,
		ignoreCreaHex?: boolean,
		attackFix?: boolean,
	) {
		if (!facefrom) {
			facefrom = this.player.flipped ? this.hexagons[this.size - 1] : this.hexagons[0];
		}

		if (
			ignoreCreaHex &&
			faceto instanceof Hex &&
			facefrom instanceof Hex &&
			this.hexagons.indexOf(faceto) != -1 &&
			this.hexagons.indexOf(facefrom) != -1
		) {
			this.facePlayerDefault();
			return;
		}

		if (faceto instanceof Creature) {
			if (faceto === this) {
				this.facePlayerDefault();
				return;
			}
			faceto = faceto.size < 2 ? faceto.hexagons[0] : faceto.hexagons[1];
		}

		if (faceto.x == facefrom.x && faceto.y == facefrom.y) {
			this.facePlayerDefault();
			return;
		}

		if (attackFix && this.size > 1) {
			//only works on 2hex creature targeting the adjacent row
			const flipOffset = this.player.flipped ? 1 : 0;
			if (facefrom.y % 2 === 0) {
				if (faceto.x - flipOffset == facefrom.x) {
					this.facePlayerDefault();
					return;
				}
			} else {
				if (faceto.x + 1 - flipOffset == facefrom.x) {
					this.facePlayerDefault();
					return;
				}
			}
		}

		const flipped = facefrom.y % 2 === 0 ? faceto.x <= facefrom.x : faceto.x < facefrom.x;

		if (flipped) {
			this.sprite.scale.setTo(-1, 1);
		} else {
			this.sprite.scale.setTo(1, 1);
		}
		this.sprite.x =
			(!flipped
				? this.display['offset-x']
				: HEX_WIDTH_PX * this.size - this.sprite.texture.width - this.display['offset-x']) +
			this.sprite.texture.width / 2;
	}

If you're new to the project, how do you call that? I think faceTo is understandable, but the other args are mysterious.

For just flipping the sprite, here's the code:

		if (flipped) {
			this.sprite.scale.setTo(-1, 1);
		} else {
			this.sprite.scale.setTo(1, 1);
		}
		this.sprite.x =
			(!flipped
				? this.display['offset-x']
				: HEX_WIDTH_PX * this.size - this.sprite.texture.width - this.display['offset-x']) +
			this.sprite.texture.width / 2;

With a different sprite setup, this can be this.sprite.scale.setTo(flipped ? -1 : 1, 1); or even better this.sprite.scale.setTo(direction, 1);

The extra code for flipping a sprite is just one consequence of the current sprite setup. It's part of a system that needs refactoring, imo. E.g., it's not currently possible just to set a creature's position and have it show up at the expected spot on screen. The only way to move a creature around is by first having a handle on a Hex, then calling a method on Creature. That's despite the fact that Creature exposes both Creature.pos and Creature.x/y.

For me, that should be easier.

But I don't have to work on it. If there's something you'd rather have me do, then feel free to say what that is.

Edit: To be clear, I'm currently working on stuff related to issues in the issues queue, but actually addressing those issues requires refactoring/rethinking some core systems. It's easier to fix e.g., a creature displaying at the wrong coordinates by coding up a new if/else branch. But it leaves the code harder to modify in the future.

I'm choosing to address what I hope are core problems behind the issues. But if you don't want me to do that, then that's fine. Just tell me.

[DreadKnight]

I'm not passionate about them, it's how things get done. You compared this project with some game jam one bear

I don't know what the bear emoji means here. If I've offended you:

lulz, it's a little bear. No worries.

I didn't compare AB to a game jam game – though if I had, comparing it to a game jam game from DeepKnight would be a compliment in my eyes. I linked to the game jam game because the source is online and I wanted to show you a system that works without needing to write down extra anchor data.

Those jam games are usually small scale and at times messy. Sad part is that most of them get forgotten right after.

You want explicit anchors and that's fine.

I really don't see how we would properly do without when we'll have lots of sprite sheets for each unit action/ability.
But yet again I could always be confused about things and perhaps even miss the point at times, communication ftw.

Also, we don't have to worry about good position regarding cardboard flipping because that's kinda pointless thing btw.

I don't want to work on changes that will be unwelcome. If you'd prefer I work on something else, then go ahead and tell me what that should be.

Since you're not on discord, I've added you to the FreezingMoon team a while ago and told you about it, but you still haven't accepted the invitation afaik. If you accept, I would be able to mention you around the repo on issues that you haven't mentioned. Anyway, I appreciate all sorts of optimizations way more than before; there was a coder that was constantly focusing on them, while I wanted more focus on things that were priority and players really cared about them, at least up to the point where this project would be less underground and would get way more eyes on it and contributions. If I can mention you on issues, I could suggest some of them that are priority at times, otherwise it's great that you can find stuff to do on your own, since you're more of a lead programmer than I would ever be.

From my point of view, above all, the codebase needs refactoring, so that common operations are easy and understandable. Drawing sprites to the screen on a particular hex and making them face a particular direction is a common operation that should be easy.

I agree with you. Though sometimes focusing on important issues like new playable units of features that we lack (online multiplayer) could boost the project in terms of popularity and get more coders that would do that refactoring way faster, just saying ;)

Here's the code for making a creature face one direction or another:

	faceHex(
		faceto: Hex | Creature,
		facefrom?: Hex | Creature,
		ignoreCreaHex?: boolean,
		attackFix?: boolean,
	) {
		if (!facefrom) {
			facefrom = this.player.flipped ? this.hexagons[this.size - 1] : this.hexagons[0];
		}

		if (
			ignoreCreaHex &&
			faceto instanceof Hex &&
			facefrom instanceof Hex &&
			this.hexagons.indexOf(faceto) != -1 &&
			this.hexagons.indexOf(facefrom) != -1
		) {
			this.facePlayerDefault();
			return;
		}

		if (faceto instanceof Creature) {
			if (faceto === this) {
				this.facePlayerDefault();
				return;
			}
			faceto = faceto.size < 2 ? faceto.hexagons[0] : faceto.hexagons[1];
		}

		if (faceto.x == facefrom.x && faceto.y == facefrom.y) {
			this.facePlayerDefault();
			return;
		}

		if (attackFix && this.size > 1) {
			//only works on 2hex creature targeting the adjacent row
			const flipOffset = this.player.flipped ? 1 : 0;
			if (facefrom.y % 2 === 0) {
				if (faceto.x - flipOffset == facefrom.x) {
					this.facePlayerDefault();
					return;
				}
			} else {
				if (faceto.x + 1 - flipOffset == facefrom.x) {
					this.facePlayerDefault();
					return;
				}
			}
		}

		const flipped = facefrom.y % 2 === 0 ? faceto.x <= facefrom.x : faceto.x < facefrom.x;

		if (flipped) {
			this.sprite.scale.setTo(-1, 1);
		} else {
			this.sprite.scale.setTo(1, 1);
		}
		this.sprite.x =
			(!flipped
				? this.display['offset-x']
				: HEX_WIDTH_PX * this.size - this.sprite.texture.width - this.display['offset-x']) +
			this.sprite.texture.width / 2;
	}

If you're new to the project, how do you call that? I think faceTo is understandable, but the other args are mysterious.

Yeah, that's quite nasty.

For just flipping the sprite, here's the code:

		if (flipped) {
			this.sprite.scale.setTo(-1, 1);
		} else {
			this.sprite.scale.setTo(1, 1);
		}
		this.sprite.x =
			(!flipped
				? this.display['offset-x']
				: HEX_WIDTH_PX * this.size - this.sprite.texture.width - this.display['offset-x']) +
			this.sprite.texture.width / 2;

With a different sprite setup, this can be this.sprite.scale.setTo(flipped ? -1 : 1, 1); or even better this.sprite.scale.setTo(direction, 1);

This would be ideal. Hopefully haven't hindered progress on that with the anchor decision somehow 🐻

The extra code for flipping a sprite is just one consequence of the current sprite setup. It's part of a system that needs refactoring, imo. E.g., it's not currently possible just to set a creature's position and have it show up at the expected spot on screen. The only way to move a creature around is by first having a handle on a Hex, then calling a method on Creature. That's despite the fact that Creature exposes both Creature.pos and Creature.x/y.

For me, that should be easier.

I see, that would be really nice to have.

But I don't have to work on it. If there's something you'd rather have me do, then feel free to say what that is.

Understood. Let me know if you accept that invite and I'll see about given time about delegating priority tasks to you.

Edit: To be clear, I'm currently working on stuff related to issues in the issues queue, but actually addressing those issues requires refactoring/rethinking some core systems. It's easier to fix e.g., a creature displaying at the wrong coordinates by coding up a new if/else branch. But it leaves the code harder to modify in the future.

Figured. Hacky fixes on top of other issues vs removing the rot altogether.

I'm choosing to address what I hope are core problems behind the issues. But if you don't want me to do that, then that's fine. Just tell me.

It's all good. I appreciate that you're wiling to change focus on request, because with the other guy that I mentioned, I could never get him to fix any specific or priority things most of the time, I kept telling him that there's a time for everything and code will most likely get revamped sooner or later anyway. This is a core problem when it comes to open source usually, as people (unpaid contributors) do only what they wanna do, but not what's really important at the time, as some of those tasks aren't really that fun and exciting usually or they have to be done on top of existing rot, which can trigger some sort of OCDs and cause grief rather.

[andretchen0]

lulz, it's a little bear. No worries.

Good to hear!

As you mentioned, people can get possessive/defensive about their ideas. I wanted to make sure that things weren't headed that way. Glad to know that's not the case. ;)

invite

I'm not sure where the invite went, but I generally don't mix social networks for data hygiene purposes. I prefer to keep my graph small.

If you'd like to point me in a direction, if it works for you, maybe use an existing issue or create a new one and then @ me.

---

For the moment, I'm working on the creature sprite refactor. To my mind, there's a fair amount of stuff to unwind around that. Systems overlap and dig into each other, e.g., when summoning a creature:

• The Priest Printer ability creates a creature and some callbacks, including fnOnSelect and passes them to HexGrid.queryHexes(). It also creates a full unmaterialized Creature and sets its alpha to 0.
• The Printer ability has its fnOnSelect callback triggered (with the arguments that already exist in the ability) when some HexGrid hexes are moused over.
• The Printer ability then calls HexGrid.previewCreature().
• HexGrid.previewCreature() creates a new Sprite for the Creature if it doesn't already exist and keeps a reference to it – despite the fact that the full unmaterialized Creature and its sprite already exists in the ability.
• Whenever the mouseover states change HexGrid hides its copy of the sprite, if it exists.
• And when the Creature is finally materialized, the Creature checks HexGrid to see if the duplicate Sprite exists and fades it out.

So, lots of knowledge of internals of other classes gets spread around and the code is brittle as a result – e.g.,

• HexGrid can't rename its materialize_overlay to tempCreatureSprite without forcing a change in Creature.
• Creature sprite placement has to be "known" by HexGrid so that it can properly place the sprite for the temporary Creature.
• HexGrid has to hand the Printer ability args back to the ability whenever there's a state change callback. Even though each callback can close over all the args when they're created in the ability.

Concretely, here's one consequence from HexGrid:

	orderCreatureZ() {
		let index = 0;
		const creatures = this.game.creatures;

		for (let y = 0, leny = this.hexes.length; y < leny; y++) {
			for (let i = 0, len = creatures.length; i < len; i++) {
				if (creatures[i].y == y) {
					this.creatureGroup.remove(creatures[i].grp);
					this.creatureGroup.addAt(creatures[i].grp, index++);
				}
			}

			if (this.materialize_overlay && this.materialize_overlay.posy == y) {
				this.creatureGroup.remove(this.materialize_overlay);
				this.creatureGroup.addAt(this.materialize_overlay, index++);
			}
		}
	}

This sorts the creatureGroup layer by y. It has to account for an exceptional posy (not pos.y) attribute on materialize_overlay. To do its job, it loops through every row, and for every row, it loops through every creature to see if they're on that row.

Under a more straightforward setup, this would be:

orderCreatureZ() {
    this.creatureGroup.sort('y');
}

[DreadKnight]

invite

I'm not sure where the invite went, but I generally don't mix social networks for data hygiene purposes. I prefer to keep my graph small.

If you'd like to point me in a direction, if it works for you, maybe use an existing issue or create a new one and then @ me.

I can't mention anyone in new issues and such unless they already had some activity on it OR are in the project's team, over here -> https://github.com/orgs/FreezingMoon/people which I sent you invitation already.

For the moment, I'm working on the creature sprite refactor. To my mind, there's a fair amount of stuff to unwind around that. Systems overlap and dig into each other, e.g., when summoning a creature:

• The Priest Printer ability creates a creature and some callbacks, including fnOnSelect and passes them to HexGrid.queryHexes(). It also creates a full unmaterialized Creature and sets its alpha to 0.
• The Printer ability has its fnOnSelect callback triggered (with the arguments that already exist in the ability) when some HexGrid hexes are moused over.
• The Printer ability then calls HexGrid.previewCreature().
• HexGrid.previewCreature() creates a new Sprite for the Creature if it doesn't already exist and keeps a reference to it – despite the fact that the full unmaterialized Creature and its sprite already exists in the ability.
• Whenever the mouseover states change HexGrid hides its copy of the sprite, if it exists.
• And when the Creature is finally materialized, the Creature checks HexGrid to see if the duplicate Sprite exists and fades it out.

Quite the list.

So, lots of knowledge of internals of other classes gets spread around and the code is brittle as a result – e.g.,

• HexGrid can't rename its materialize_overlay to tempCreatureSprite without forcing a change in Creature.
• Creature sprite placement has to be "known" by HexGrid so that it can properly place the sprite for the temporary Creature.
• HexGrid has to hand the Printer ability args back to the ability whenever there's a state change callback. Even though each callback can close over all the args when they're created in the ability.

Concretely, here's one consequence from HexGrid:

	orderCreatureZ() {
		let index = 0;
		const creatures = this.game.creatures;

		for (let y = 0, leny = this.hexes.length; y < leny; y++) {
			for (let i = 0, len = creatures.length; i < len; i++) {
				if (creatures[i].y == y) {
					this.creatureGroup.remove(creatures[i].grp);
					this.creatureGroup.addAt(creatures[i].grp, index++);
				}
			}

			if (this.materialize_overlay && this.materialize_overlay.posy == y) {
				this.creatureGroup.remove(this.materialize_overlay);
				this.creatureGroup.addAt(this.materialize_overlay, index++);
			}
		}
	}

This sorts the creatureGroup layer by y. It has to account for an exceptional posy (not pos.y) attribute on materialize_overlay. To do its job, it loops through every row, and for every row, it loops through every creature to see if they're on that row.

Under a more straightforward setup, this would be:

orderCreatureZ() {
    this.creatureGroup.sort('y');
}

That would be ideal.

[andretchen0]

Since you're not on discord, I've added you to the FreezingMoon team a while ago and told you about it, but you still haven't accepted the invitation afaik.

Ah, yes. You'd mentioned that. My bad. I found the invite, but it's expired. Resend and I'll accept.

[DreadKnight]

Since you're not on discord, I've added you to the FreezingMoon team a while ago and told you about it, but you still haven't accepted the invitation afaik.

Ah, yes. You'd mentioned that. My bad. I found the invite, but it's expired. Resend and I'll accept.

Alright, found the thingy and sent it again. This is needed because GitHub is very siloed unlike Twitter in order to prevent abuse and spam; also to make things more efficient.

[andretchen0]

Joined