AncientBeast icon indicating copy to clipboard operation
AncientBeast copied to clipboard

Creature Dash: arrow keys don't move the "active" grid element

Open andretchen0 opened this issue 2 years ago • 3 comments

Below, "Uncle Fungus" is the "active" grid element.

Screenshot 2023-08-08 at 14 49 06 copy

Using arrow keys does not move the "active" element in the grid, leading to a mismatch like this:

Screenshot 2023-08-08 at 14 53 26

Here, the "active" grid element is "Uncle Fungus", but "Scavenger" is shown on the left.

To Reproduce

  1. Start a new game
  2. Click on the Priest's summon ability
  3. In the creature dash, hover over a creature thumbnail – it will become "active"
  4. Use arrow keys – the creature shown on the right of the screen will change, but the "active" grid element will not

Expected behavior

The "active" grid element should match what's shown on the right side of the screen.

andretchen0 avatar Aug 08 '23 12:08 andretchen0

I'm aware of this one, no open issue for it already though afaik. I was thinking that this wasn't easily fixable at the time.

DreadKnight avatar Aug 08 '23 13:08 DreadKnight

I was thinking that this wasn't easily fixable at the time.

The whole thing is in need of a rethink/refactor, I think. To give you an idea:

The Creature Grid / Dash isn't isolated, either as a module or otherwise. It's a collection of attributes and methods in interface.js and there's a fair amount of leakage. E.g., the Dash system works by setting an attribute on interface that's picked up during button presses, to then be sent to the Priest's materialize method.

And here's how the priest ability says "choose a creature to summon":

	// Ask the creature to summon
	G.UI.materializeToggled = true;
	G.UI.toggleDash('randomize');

... so it's got to muck around with UI internals, despite not having a direct reference to UI. And then it doesn't even get a direct reference back. Some part of interface has to in turn muck around in the ability like so:

	activeCreature.abilities[3].materialize(this.lastViewedCreature);

Not pointing fingers, but just to walk you through what's going on: here are the assumptions behind and code brittleness introduced with just that single line:

  • activeCreature has to have a field called abilities.
  • That field has to be an array that has at least 4 slots.
  • The 3 slot in the array has to have a materialize method.

And that doesn't include how it is that interface decides that this activeCreature is a creature who can summon.

The whole thing could be a lot less brittle and a lot less complicated if it were called like this:

	// Ask the creature to summon
	G.chooseCreatureTypeFrom(this.creature.getSummonableCreatureTypes()).then(type => this.materialize(type));
  • No special interface code to figure out if the current creature can summon and is currently summoning.
  • No knowledge of interface at all in the ability.

andretchen0 avatar Aug 08 '23 14:08 andretchen0

I have a coded solution. I'll wait on feedback from this discussion before finalizing and opening a PR.

andretchen0 avatar Aug 13 '23 01:08 andretchen0