Creature delay bugfix and refactor, closes #2158 #2275 #2426

andretchen0 · 2023-07-16T10:21:07Z

bugfix: fix creature delay reset and delays across rounds

splits creature.delay() into creature.hinder() and creature.wait()
creature.hinder() is called when an attacker delays the creature
creature.wait() is called when the player delays their own creature
adds tests for hinder, wait and related getters
disallows setting of creature.delayable and creature.delayed
simplifies src/creature_queue.ts; its values are now derived on the fly – no need to call update()
adds types and cleans up ui/queue.ts, removing stopGap functions needed to derive correct data from buggy delays

Creature.delay()

Creature delayed/delayable were set in different parts of the code. And the Creature class treated delay()/delayed as monolithic – however there were 2 distinct cases in code:

delays from the player deciding their own creature should "wait". These delays do not persist across rounds.
delays resulting from attacks from other players – "hindering" the creature. These delays do persist across rounds.

These two cases are now separate methods: wait() and hinder(), respectively.

delayable was converted to canWait and is no longer settable – it's derived.

CreatureQueue

CreatureQueue (creature_queue.ts) kept its own state related to creature delay status and turn position. That had to be kept updated and in sync with the world. Most of its methods have been removed. It now sufficient to change the creatures themselves. CreatureQueue no longer holds any data. It merely derives it, so it's always up to date.

Future

game does a lot of UI calls, e.g., setting button states. Lots of care (or trial and error) is needed to tune setTimeouts so that the correct UI state ends up on screen. Using the current paradigms in the codebase, it would be
simpler if game communicated the possible need for a UI update by dispatching an event. Then the UI could derive its state from the game.

Likewise, interface.js does things like check if a button press should have an action, e.g., if btnDelay is pressed, it checks whether the game.activeCreature can be delayed. These kinds of business rules shouldn't reside in the UI. It would be simpler if it merely fired off the button press and let game or the creature itself decide how to respond – the OOP principle of "tell, don't ask".

* Split creature.delay() into creature.hinder() and creature.delay() * creature.hinder() is called when an attacker delays the creature * creature.wait() is called when the player delays their own creature * simplified creature queue See #2158, #2275

vercel · 2023-07-16T10:21:10Z

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

Name	Status	Preview	Updated (UTC)
ancientbeast	✅ Ready (Inspect)	Visit Preview	Jul 18, 2023 8:36pm

ghost · 2023-07-16T10:21:53Z

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

Legend

andretchen0 · 2023-07-17T14:44:06Z

Any chance this will be looked at/merged soon? I've got other code in the works, but it uses this branch as a starting point.

DreadKnight · 2023-07-17T17:45:22Z

Yeah, getting back at my desk in a bit and starting to catch up with work again.

…

On Mon, Jul 17, 2023, 5:44 PM andretchen0 ***@***.***> wrote: Any chance this will be looked at/merged soon? I've got other code in the works, but it uses this branch as a starting point. — Reply to this email directly, view it on GitHub <#2426 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEPNX26MTYV7GGV5WIFYDDXQVFUBANCNFSM6AAAAAA2L3GXIM> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

DreadKnight · 2023-07-17T23:22:48Z

Regression: mini-tutorial is missing

andretchen0 · 2023-07-18T00:09:50Z

I'm not sure if that's the right fix.

In the old version of the code, in nextCreature() ...

The old code depended on this.queue.isCurrentEmpty() to trigger a nextRound() and return early. (Because the the old queue needed to be explicitly updated, but hadn't been updated yet, it was empty – even though there are two priests already materialized.)
nextRound would increment game.turn from 0 -> 1.
On the next loop – which became the first actual round of gameplay, the mini-tutorial code condition this.turn === 1 would return true and show the tutorial.

Now ...

The CreatureQueue doesn't get updated. It's just a few functions that read from game.creatures and return. So we can't rely on the edge case that the starting queue is empty even if there are game.creatures – that edge case doesn't exist anymore.

So ...

I changed the condition to this.turn === 0.

But the this.turn should initialized as 1 instead. Otherwise, round numbering will start at 0 and shown to the player.

Do you have a preference?

DreadKnight · 2023-07-18T00:15:30Z

@andretchen0 Usually fighting games have an announcer be like "Round 1, Fight!" and then the gameplay starts.

There's also some more silliness like that regarding counting players, so that P1 ends up with id 0 if I recall right.

andretchen0 · 2023-07-18T00:23:02Z

@andretchen0 Usually fighting games have an announcer be like "Round 1, Fight!" and then the gameplay starts.

There's also some more silliness like that regarding counting players, so that P1 ends up with id 0 if I recall right.

Not everywhere, but there's often a translation that happens from 0-indexing to 1-indexing when going from programmer-space to user-space.

That's my preference frankly, but there are probably bigger fish to fry. I'm not sure what poking at game.turn will do. There are always surprises.

DreadKnight · 2023-07-18T00:34:50Z

@andretchen0 Usually fighting games have an announcer be like "Round 1, Fight!" and then the gameplay starts.
There's also some more silliness like that regarding counting players, so that P1 ends up with id 0 if I recall right.

Not everywhere, but there's often a translation that happens from 0-indexing to 1-indexing when going from programmer-space to user-space.

That's my preference frankly, but there are probably bigger fish to fry. I'm not sure what poking at game.turn will do. There are always surprises.

Well, we could consider first one round 0, as it's a standard thingy, with Dark Priests only and probably no units acting, unless having some special passive ability to avoid materialization sickness (might happen eventually) or some alternative summoner type at some point that that can heal the sickness. I haven't found any other issues so far, might merge soon.

DreadKnight · 2023-07-18T00:51:57Z

@andretchen0 Your last patch somehow didn't changed things 😆

andretchen0 · 2023-07-18T00:54:33Z

@andretchen0 Your last patch somehow didn't changed things 😆

Ok. I'll have a look tomorrow.

andretchen0 · 2023-07-18T11:24:45Z

Ah, gotcha. I didn't have a clear understanding of what was missing.

If I understand correctly, what was missing after the last fix was that "Round 1" wasn't shown in the "stringConsole" on the first round – before the mini tutorial. Everything else was fine?

With this latest commit, "Round 1" is shown before the mini tutorial.

DreadKnight · 2023-07-18T20:19:46Z

Ah, gotcha. I didn't have a clear understanding of what was missing.

If I understand correctly, what was missing after the last fix was that "Round 1" wasn't shown in the "stringConsole" on the first round – before the mini tutorial. Everything else was fine?

With this latest commit, "Round 1" is shown before the mini tutorial.

I was talking about the unit queue, in beta.ancientbeast.com the first round marker shows "Round 2"; good catch though.

andretchen0 · 2023-07-18T20:36:15Z

I was talking about the unit queue, in beta.ancientbeast.com the first round marker shows "Round 2"; good catch though.

Ok. Got it.

DreadKnight · 2023-07-18T21:14:34Z

I was talking about the unit queue, in beta.ancientbeast.com the first round marker shows "Round 2"; good catch though.

Ok. Got it.

Still shows "Round 1" to me, unlike the beta.

Unless we go with it like this and consider first thing ever round 0, heh.

andretchen0 · 2023-07-18T21:29:10Z

I guess I'm not following.

Option A

Here's how it was:

https://ancientbeast-qt9ebrojo-freezingmoon.vercel.app/

This shows "Round 2" in the round marker on game start – I guess this assumes that the round marker marks the start of a round.

Option B

Here's how it is currently:

https://ancientbeast-byxfjd48f-freezingmoon.vercel.app/

This shows "Round 1" in the round marker on game start - I guess this assumes that the round marker marks the end of a round.

Is either Option A or Option B correct? If so, which one?

Otherwise, what should the round marker read at the beginning of the game?

DreadKnight · 2023-07-18T21:41:05Z

I guess I'm not following.

Option A

Here's how it was:

https://ancientbeast-qt9ebrojo-freezingmoon.vercel.app/

This shows "Round 2" in the round marker on game start – I guess this assumes that the round marker marks the start of a round.

Option B

Here's how it is currently:

https://ancientbeast-byxfjd48f-freezingmoon.vercel.app/

This shows "Round 1" in the round marker on game start - I guess this assumes that the round marker marks the end of a round.

Is either Option A or Option B correct? If so, which one?

Otherwise, what should the round marker read at the beginning of the game?

It used to be good, though it wasn't displaying properly for me for some reason.

Anyway, interesting point of view, will think about it and pick one, though queue is for things to come, but we would be lacking some "Round 1" marker otherwise, hmm...

DreadKnight · 2023-07-18T23:42:55Z

Will leave it like this for now (option B), as I'll think of the marker like a tombstone for the round 🪦 "Here lies round 2"

andretchen0 added 2 commits July 16, 2023 12:16

test: add tests for creature.wait() .hinder() and delay getters

b7fbb77

vercel bot deployed to Preview July 16, 2023 10:21 View deployment

andretchen0 marked this pull request as ready for review July 16, 2023 10:23

style: remove empty arrow function

b5e5e8d

vercel bot deployed to Preview July 16, 2023 10:33 View deployment

andretchen0 added 2 commits July 16, 2023 12:52

test: fix game mock

d3b0546

test: add extra expect()

76713f3

vercel bot deployed to Preview July 16, 2023 10:54 View deployment

andretchen0 added 2 commits July 17, 2023 00:38

refactor: Creature Queue remove update(), always derive

802c4e5

refactor: ui/queue add types, improve comments, eliminate workarounds

7299aee

vercel bot deployed to Preview July 16, 2023 22:43 View deployment

fix: mini tutorial log

ca378c0

vercel bot deployed to Preview July 17, 2023 23:45 View deployment

fix: show user round numbers starting at 1

ca54a61

vercel bot deployed to Preview July 18, 2023 00:42 View deployment

fix: log Round for first turn

c600fbe

vercel bot deployed to Preview July 18, 2023 11:23 View deployment

fix: interface queue displayed round number off by 1

34d5b15

vercel bot deployed to Preview July 18, 2023 20:36 View deployment

DreadKnight merged commit e138187 into FreezingMoon:master Jul 18, 2023

andretchen0 deleted the creature-delay-merge branch July 19, 2023 00:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creature delay bugfix and refactor, closes #2158 #2275 #2426

Creature delay bugfix and refactor, closes #2158 #2275 #2426

andretchen0 commented Jul 16, 2023 •

edited

Loading

vercel bot commented Jul 16, 2023 •

edited

Loading

ghost commented Jul 16, 2023 •

edited by ghost

Loading

Legend

andretchen0 commented Jul 17, 2023

DreadKnight commented Jul 17, 2023 via email

DreadKnight commented Jul 17, 2023

andretchen0 commented Jul 18, 2023

DreadKnight commented Jul 18, 2023

andretchen0 commented Jul 18, 2023

DreadKnight commented Jul 18, 2023 •

edited

Loading

DreadKnight commented Jul 18, 2023

andretchen0 commented Jul 18, 2023

andretchen0 commented Jul 18, 2023

DreadKnight commented Jul 18, 2023 •

edited

Loading

andretchen0 commented Jul 18, 2023 •

edited

Loading

DreadKnight commented Jul 18, 2023

andretchen0 commented Jul 18, 2023

DreadKnight commented Jul 18, 2023 •

edited

Loading

Option A

Option B

DreadKnight commented Jul 18, 2023

Creature delay bugfix and refactor, closes #2158 #2275 #2426

Creature delay bugfix and refactor, closes #2158 #2275 #2426

Conversation

andretchen0 commented Jul 16, 2023 • edited Loading

Creature.delay()

CreatureQueue

Future

vercel bot commented Jul 16, 2023 • edited Loading

ghost commented Jul 16, 2023 • edited by ghost Loading

Legend

andretchen0 commented Jul 17, 2023

DreadKnight commented Jul 17, 2023 via email

DreadKnight commented Jul 17, 2023

andretchen0 commented Jul 18, 2023

DreadKnight commented Jul 18, 2023

andretchen0 commented Jul 18, 2023

DreadKnight commented Jul 18, 2023 • edited Loading

DreadKnight commented Jul 18, 2023

andretchen0 commented Jul 18, 2023

andretchen0 commented Jul 18, 2023

DreadKnight commented Jul 18, 2023 • edited Loading

andretchen0 commented Jul 18, 2023 • edited Loading

DreadKnight commented Jul 18, 2023

andretchen0 commented Jul 18, 2023

Option A

Option B

DreadKnight commented Jul 18, 2023 • edited Loading

Option A

Option B

DreadKnight commented Jul 18, 2023

andretchen0 commented Jul 16, 2023 •

edited

Loading

vercel bot commented Jul 16, 2023 •

edited

Loading

ghost commented Jul 16, 2023 •

edited by ghost

Loading

DreadKnight commented Jul 18, 2023 •

edited

Loading

DreadKnight commented Jul 18, 2023 •

edited

Loading

andretchen0 commented Jul 18, 2023 •

edited

Loading

DreadKnight commented Jul 18, 2023 •

edited

Loading