Make (equivalent for) dragAndMoveBall available again #15

igrep · 2024-06-05T13:43:34Z

~~This is still draft because I left a several things below and I want to show you before completing to reduce the risk in case of refusal.~~ FEEDBACK WELCOME!

~~More customizable debugging mode: make controlMode and visualizeBlocks separately configurable on popup.html.~~ See Make (equivalent for) dragAndMoveBall available again #15 (comment) on why I removed this.
Refactor: apply updateBallPositionTo to other objects if possible and it deserves.

Background

When I first play with brick-block-anywhere on my website, I found a bug that could be a good chance for me to learn the browser's behavior. This feature is very useful for reproducing the bug and testing after fixing it.

Changes Summary

Add a function controlBallByMouse to move the ball by mouse so that we can easily put the ball at some place for debugging.
Abstract the procedure to move the ball (updateBallPositionBy and updateBallPositionTo) to hide the details and for consistency.
Merge TestMessage into StartMessage because now the non-debugging mode has to know whether the debugging mode is enabled to disable the default movement of the ball.
- Bonus: this change also makes 12c7f53 unnecessary. It's simpler!

Screenshot

This reverts commit 12c7f53.

Background ==== When I first play with brick-block-anywhere on [my website](https://the.igreque.info/), I found a bug that could be a good chance for me to learn the browser's behavior. This feature is very useful for reproducing the bug and testing after fixing it. Changes Summary ==== - Add a function `controlBallByMouse` to move the ball by mouse so that we can easily put the ball at some place for debugging. - Abstract the procedure to move the ball (`updateBallPositionBy` and `updateBallPositionTo`) to hide the details and for consistency. - Merge `TestMessage` into `StartMessage` because now the non-debugging mode has to know whether the debugging mode is enabled to disable the default movement of the ball. - Bonus: this change also makes canalun@12c7f53 unnecessary. It's simpler! Next ==== - More customizable debugging mode: make `controlMode` and `visualizeBlocks` separately configurable on popup.html. - Refactor: apply `updateBallPositionTo` to other objects if possible and it deserves.

canalun · 2024-06-21T16:51:01Z

src/game/configuration/settings.ts

+  sound: boolean,
+  visualizeBlocks: boolean,
+  controlMode: "normal" | "mouse"
+}


[imo] How about a structure like this?👀 I think it might be more robust.

Suggested change

sound: boolean,

visualizeBlocks: boolean,

controlMode: "normal" | "mouse"

}

sound: boolean

} & {

visualizeBlocks: false,

controlMode: "normal"

} | {

visualizeBlocks: true,

controlMode: "mouse"

}

🤔 I'm undecided about this.

I found two bugs using the controlMode: "mouse" feature. I actually couldn't find one of the bugs actually because the borders drawn by visualizeBlocks prevent me from reproducing it. That's why I left the TODO "More customizable debugging mode: make controlMode and visualizeBlocks separately configurable on popup.html.".

But now, I know how to fix it and the change to fix it would perhaps remove the root problem of visualizeBlocks. So separating them could just make the code more complex.

I beg your opinion!

NOTE: I'll send another PR to fix the bug mentioned here today or tomorrow!

Rethinking this problem, now I want to keep as is.

As my need for separating the debug options has already been satisfied by (manually editing the code), we would rarely need to switch them independently. So keep popup.tsx simple by not adding new <input>s for the time being.

But IMO visualizeBlocks and controlMode are essentially different. They should be configured independently (even by temporarily updating the code). Your suggestion makes it a little harder to do that.

canalun · 2024-06-21T16:54:20Z

src/game/configuration/settings.ts

+  controlMode: "normal" | "mouse"
+}
+
+export function disableScoreboardIfDebug(options: StartOptions): StartOptions {


[Q] Is it possible to remove this function and make the scoreboard disabeld for the debug mode within sendMessageToIsolatedWorldOnActiveTabForTest() in popup.tsx? I'd like to hear your opnion :)

👍 Indeed. I'm a little worried that'd make popup.tsx complex, but more user-friendly!

It seems I mistook your comment wrong 😅 . But 2614729 would surpass your expectation!

src/game/animation/updateBall.ts

canalun · 2024-06-21T17:17:32Z

@igrep
Apologies for my late reply. After a busy month, now I have time to review this PR.
I like your idea! So could you please go for it? Sorry for late response.
(FYI. I've left some comments, thank you for checking them!)

canalun · 2024-06-21T17:25:36Z

And I like your branch name :)

igrep · 2024-06-22T00:46:21Z

And I like your branch name :)

Thank you! 😁 To tell the truth, I couldn't make up a good name for controlMode: "mouse" at first. Then I decided to give a name that sounds something super because it breaks the nature of the brick-break game. "God mode" might be more suitable, but "God" could be so religious that I couldn't determine it's safe. "Ninja" would have no such a problem and everyone loves it by contrast!

And the prefix "2" means it's just the second try 😅 .

- Disable scoreboard on UI when the debug mode is enabled, then delete `disableScoreboardIfDebug`. - Answer to [a review comment from canalun](canalun#15 (comment)). - It seems I actually took the comment wrong, but this would make it easier to add independent debug options. - Abstract the common steps in `sendMessage*` functions. - Add the "Start" button to the bottom - This makes easier to start when enabling the debug mode.

canalun

sorry for the delay. I'll review the other PRs also soon...!!

src/game/animation/updateObject.ts

canalun · 2024-07-23T06:14:04Z

src/popup.tsx

      ) : null}
+      <br />
+      <button onClick={start}> Start! </button>


It seems that the start button is duplicated unnecessarily?👀

As referred in 2614729, it's intentional.
I think now the settings menu is long enough that putting the start button both at the top and the bottom is a good choice.

Co-authored-by: canalun <[email protected]>

igrep · 2024-07-23T08:25:38Z

NOTE: I force-pushed 3aad11f because 8077019 doesn't work because only the declaration of ball is renamed.

canalun

thank you so much!!!!!

igrep added 3 commits June 1, 2024 18:37

Revert "Keep debug-or-start between replays"

f41b455

This reverts commit 12c7f53.

Add a TODO

a286c54

canalun reviewed Jun 21, 2024

View reviewed changes

src/game/animation/updateBall.ts Outdated Show resolved Hide resolved

igrep mentioned this pull request Jun 23, 2024

Fix: Blocks shifts by the border's size when removed or debug mode enabled #17

Merged

igrep added 2 commits June 24, 2024 20:43

Abstract the function to move ball and bar

f29abee

igrep marked this pull request as ready for review June 24, 2024 12:44

igrep mentioned this pull request Jun 29, 2024

Fix: Blocks of multi-line inline elements are too large #18

Merged

canalun reviewed Jul 23, 2024

View reviewed changes

Rename the argument as the function name

3aad11f

Co-authored-by: canalun <[email protected]>

igrep force-pushed the ninja2 branch from 8077019 to 3aad11f Compare July 23, 2024 08:01

canalun approved these changes Jul 23, 2024

View reviewed changes

canalun merged commit 7283625 into canalun:main Jul 23, 2024

igrep deleted the ninja2 branch July 23, 2024 20:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make (equivalent for) dragAndMoveBall available again #15

Make (equivalent for) dragAndMoveBall available again #15

igrep commented Jun 5, 2024 •

edited

Loading

canalun Jun 21, 2024 •

edited

Loading

igrep Jun 22, 2024 •

edited

Loading

igrep Jun 24, 2024

canalun Jun 21, 2024

igrep Jun 22, 2024

igrep Jun 24, 2024

canalun commented Jun 21, 2024

canalun commented Jun 21, 2024

igrep commented Jun 22, 2024

canalun left a comment

canalun Jul 23, 2024

igrep Jul 23, 2024

igrep commented Jul 23, 2024

canalun left a comment

Make (equivalent for) dragAndMoveBall available again #15

Make (equivalent for) dragAndMoveBall available again #15

Conversation

igrep commented Jun 5, 2024 • edited Loading

Background

Changes Summary

Screenshot

canalun Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

igrep Jun 22, 2024 • edited Loading

Choose a reason for hiding this comment

igrep Jun 24, 2024

Choose a reason for hiding this comment

canalun Jun 21, 2024

Choose a reason for hiding this comment

igrep Jun 22, 2024

Choose a reason for hiding this comment

igrep Jun 24, 2024

Choose a reason for hiding this comment

canalun commented Jun 21, 2024

canalun commented Jun 21, 2024

igrep commented Jun 22, 2024

canalun left a comment

Choose a reason for hiding this comment

canalun Jul 23, 2024

Choose a reason for hiding this comment

igrep Jul 23, 2024

Choose a reason for hiding this comment

igrep commented Jul 23, 2024

canalun left a comment

Choose a reason for hiding this comment

igrep commented Jun 5, 2024 •

edited

Loading

canalun Jun 21, 2024 •

edited

Loading

igrep Jun 22, 2024 •

edited

Loading