Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use P1 inverted color to mark selected bar in editor #870

Merged

Conversation

DeinAlptraum
Copy link
Contributor

Before, the selected bar would use P2's color to mark the selected bar. This leads to no marking at all when no color has been selected for P2.

The color attribute for each bar currently holds only a single integer for the player number, since this reuses the coloring-mechanism of the actual sing mode. I chose to mark the selected syllable as belonging to player 99 (instead of player 2) and then making a distinction for that number. The choice is arbitrary, I just didn't want to use 0 since it seems to be already used for something else.

Currently, the color used for marking is the inversion of P1's color. We could also change this to not be based on the player colors at all, and instead use two fixed colors for selected/not selecte or similar.

Copy link
Member

@barbeque-squared barbeque-squared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with hardcoding/(ab)using 99 to be this color (otherwise it's probably going to explode in PR size) however I would request the following changes:

  • define a constant eg INVERTED_1 = 99 in UDraw (use a better name if you have one)
  • Use that constant instead of hardcoding 99 in UDraw and UScreenEditSub

makes things more self-explanatory when someone looks at this code in a year's time or so.

@DeinAlptraum
Copy link
Contributor Author

That's a good idea, changed

@barbeque-squared barbeque-squared self-requested a review July 18, 2024 14:53
@barbeque-squared barbeque-squared self-requested a review July 21, 2024 12:40
@barbeque-squared
Copy link
Member

Took me a while to realize that 870.patch is not the same as 870.diff. Which makes sense I guess.
And the duet stuff is now also consistent, very nice, thanks!

I've opened #873 to keep track of the whitespace.

@barbeque-squared barbeque-squared merged commit 37ed646 into UltraStar-Deluxe:master Jul 21, 2024
4 checks passed
@DeinAlptraum DeinAlptraum deleted the fix-editor-bar-color branch July 21, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants