GNOME Bugzilla – Bug 736693
Scoreboard icons should match game theme
Last modified: 2015-02-16 18:30:14 UTC
Icons on the top right scoreboard should match the ones from the current tile set. More info here: https://bugzilla.gnome.org/show_bug.cgi?id=664983#c27
The need of having multiple themes, apart a (todo) highcontrast one, may need discussions.
They're harmless! No need to remove them.
<irony><p>Let’s add a font-chooser in Sudoku in this case!</p></irony> People launching a board game are usually not there to customize the view, they want it to “just be cool” (the same thing as “just work” for code, but for artworks). And if there is themes, they want genius things that *really* change the look&feel, including the background[1], or the grid (using an extended alquerque’s board, to test!), the type of animations, and so on. More advanced in computing people don’t understand why they couldn’t add one new as the code exists (the bug exists in Four-in-a-row[2], but same applies here). Having themes could make sense in Mahjongg for example, as an important part of the game is to find tiles with same drawing; same in a Memory game, etc., but I don’t see how it could be the case here. Another problem with the themes is that their name is not translated (the bug exists in Five-or-more[3], but same applies here again). Let’s just remove that, we have more useful code to write (cleanings, keyboard play as in Taquin, start screen, <irony>font-chooser in Sudoku</irony>…). [1] Don’t tell me changing background color is doable ‘via’ hacking the svgs, that is not the good code design if we want for example to highlight a cell on hover/click/hint request/etc., or implement moving with keyboard. [2] https://bugzilla.gnome.org/show_bug.cgi?id=307446 [3] https://bugzilla.gnome.org/show_bug.cgi?id=672675
These are all good points, but I'm not really convinced they warrant removing the feature that already exists. * Translations should be handled with a lookup table for the themes that we ship by default: https://git.gnome.org/browse/gnome-mahjongg/tree/data/translatable_game_names.h * It would indeed be better not to put the background color in the SVGs, but it's also not a big deal. Both of our themes now have the same background color, so our code for highlighting the squares can assume that. Note that I recently deleted all except one of Iagno's alternate themes, and it's definitely not a big deal either way.
To be clear: when I say "it's not a big deal," in this case I mean "you can remove the themes if you really want to." The white sun piece in our only alternate theme is too hard to see anyway.
Eh, there's also bug #606374. This just seems like something that might irritate players with no significant benefit to us.
And like you mentioned, it'd be good to support high contrast themes as well... I think bug #735053 was a mistake, we shouldn't have put dark.svg and light.svg into a gresource as long as themes are still supported; unless we're going to completely remove themes, that'll have to be reverted to fix this bug.
I removed the ability to change themes from the GUI. I'm super open to bringing back themes, but at this point somebody else has to do that work.
Reopened since we brought back themes without fixing this
Created attachment 296922 [details] [review] Scoreboard, first try. I worked on it for the fun (it was a quite technical and interesting part I think), but the code I wrote is a little complicated and hacky, and the try I gave for animating the cursor was a big fail as it was slowing completely the rendering. If someone could have a look or even rewrite it a better way…
Created attachment 296937 [details] [review] Add property to define the color of scoreboard's mark for each theme As discussed with Arnaud
Created attachment 296940 [details] [review] Change color for the Classic theme to gray
The two patches has been pushed. There’s no animation (for now) but that could be added in the future, as it’s based on a DrawingArea. For now, the code is a little hacky, more work (and some cleaning) needed in the next cycle. I close this bug as resolved, but feel free to continue working in this area. Thanks everyone. =)