FreezingMoon / AncientBeast

The Turn Based Strategy Game/eSport. Master your beasts! 🐺
https://AncientBeast.com
GNU Affero General Public License v3.0
1.69k stars 589 forks source link

Frogger Jump location preview [bounty: 4 XTR] #2574

Closed DreadKnight closed 6 months ago

DreadKnight commented 7 months ago

As follow-up to issue #2533 's fix, Uncle Fungus's Frogger Jump ability could show up transparent cardboard preview to the hovered target location

gg447062 commented 7 months ago

I'm glad to do this one

DreadKnight commented 7 months ago

Sure, will assign soon.

On Tue, Apr 30, 2024, 9:22 PM Terminalman @.***> wrote:

I'm glad to do this one

— Reply to this email directly, view it on GitHub https://github.com/FreezingMoon/AncientBeast/issues/2574#issuecomment-2086416935, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEPNX3J72F6CEKT65PNYXTY77OMXAVCNFSM6AAAAABGYSUDHWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBWGQYTMOJTGU . You are receiving this because you authored the thread.Message ID: @.***>

gg447062 commented 6 months ago

@DreadKnight this one seems good to go, but just a quick question. At the bottom of the hexgrid code I found a function called fadeOutTempCreature that has a comment above it mentioning factoring it out but it seems to do exactly what we need when using the previewCreature function so I used it for this ability. Does this seem ok?

DreadKnight commented 6 months ago

fadeOutTempCreature

@gg447062 Took a peek at it. Well, ideally it would be to do things as indicated in the comment there it seems 🐻 You could use that and I could open new issue regarding the TODO refactoring 🤔 hopefully it won't create chaos.

https://github.com/FreezingMoon/AncientBeast/blob/159d4df58b8e3d3d3599ac781be0302cb5586056/src/utility/hexgrid.ts#L1669

gg447062 commented 6 months ago

Ok from what I can tell, that function is only used twice so I don't think it should cause too much chaos to refactor it. I think the thing that confused me about the comment was that it mentions the existing temp creature created by /src/abilities/Dark-Priest.js but unless I'm totally off the temp creature is this.materialize_overlay, which is also not an instance of Creature.creatureSprite.

DreadKnight commented 6 months ago

Ok from what I can tell, that function is only used twice so I don't think it should cause too much chaos to refactor it. I think the thing that confused me about the comment was that it mentions the existing temp creature created by /src/abilities/Dark-Priest.js but unless I'm totally off the temp creature is this.materialize_overlay, which is also not an instance of Creature.creatureSprite.

@gg447062 Comment could be wrong, I'm not sure. Feel free to poke at it and make a PR 🐻

gg447062 commented 6 months ago

@DreadKnight Ok sounds good, I'm going to submit a PR for this one for now.

DreadKnight commented 6 months ago

Fixed in ##2580