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

add new option in song screen menu that exits and mark the song broken #978

Merged
merged 1 commit into from
May 26, 2024

Conversation

twollgam
Copy link
Contributor

@twollgam twollgam commented Mar 25, 2024

What does this PR do?

Add new option in song screen menu that exits and mark the song broken.
The information is stored in the database and can later be used for song filtering

Closes Issue(s)

#942

Motivation

Many songs from usdb are not synchronous so the are not performable. This is the first step to be able to filter out those songs. Later #868 will be extended to apply a filter for broken songs.

Testing

Actually there is nothing that the user can notice after a song is marked. Tests can only prove that the database.xml holds the broken flag.
On Windows the database.xml ist found at C:\Users\<username>\AppData\Roaming\performous.

@twollgam twollgam changed the title add new option in song screen menu that exists and mark the song broken add new option in song screen menu that exits and mark the song broken Mar 25, 2024
@Baklap4
Copy link
Member

Baklap4 commented Mar 30, 2024

Is this to be reviewed? Or is it still in draft phase?

@twollgam twollgam marked this pull request as ready for review March 30, 2024 10:18
the information is stored in the database and can later be used for song filtering
@HetorusNL
Copy link
Member

The implementation looks good to me. I would, however, suggest to rename 'Abort' to something more explicitly suggesting that this is 'marking the song as broken'.

Don't know if I'm necroing something, but the whole tabs vs spaces, what's the intended style for Performous? And if there is a style, it might be a good idea to (auto?) format all files and get it over with (and at the same time invalidating all PR's if github can't handie this..)

@twollgam
Copy link
Contributor Author

I would, however, suggest to rename 'Abort' to something more explicitly suggesting that this is 'marking the song as broken'.

Any suggestions?

  1. Abort cause defect
  2. Abort defect song
  3. Mark song defect and quit
  4. ...

@twollgam
Copy link
Contributor Author

Don't know if I'm necroing something, but the whole tabs vs spaces, what's the intended style for Performous? And if there is a style, it might be a good idea to (auto?) format all files and get it over with (and at the same time invalidating all PR's if github can't handie this..)

You mean indent? There is a PR for rejecting wrong formatted files locally. I think github gives no support server side. But a pipeline could fail for wrong formatted files.

@HetorusNL
Copy link
Member

HetorusNL commented May 24, 2024

I would, however, suggest to rename 'Abort' to something more explicitly suggesting that this is 'marking the song as broken'.

Any suggestions?

1. Abort cause defect

2. Abort defect song

3. Mark song defect and quit

4. ...

Edit:
There is also the Exit to song browser and mark song broken tooltip thingy, I forgot, in that case simply Abort might be good as well

Maybe something like: Quit (Song Defect) or even Quit (Defect) as the other menu options are also very short, but I'm not sure

@twollgam twollgam merged commit da1f6e4 into performous:master May 26, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants