Added row highlighting based on chosen tag

[shihan42]

When choosing one of the first four tags, the corresponding row and its children are highlighted in the tag's color. However, if a child row possesses a different tag, it is highlighted in the color corresponding to that specific tag instead.

I tried to stay true to the original code with minimal refactoring. Considering the somewhat state-based rendering approach, it became imperative to designate rows for highlighting to ensure accurate colorization during subsequent passes. This explains the approach taken in the code within ProductionTableFlatHierarchy.cs.

At the moment, I'm not quite satisfied with all the hardcoded color stuff in YAFCui/Core/Structs.cs and YAFCui/Rendering/RenderingUtils.cs. However, given the substantial effort required for refactoring, I refrained from doing so at this time.

I tagged you for review, @veger, because you implemented the color coding for overproduction, and I want to ensure that we don't encounter any regressions :-)

Closes #59

[shpaass]

Main functionality fits the description.

Moving to testing overproduction.

[shpaass]

Turns out overproduction itself has a bug, but I noticed no bugs related to the current PR. Below is the example to confirm this fact.

Moving to inspecting the code.

[shpaass]

I looked at the code, and it seems to be fine, but someone else needs to check if the things are where they should be architecture-wise.

[veger]

Sorry for the wall of text and the amount of comments. I do like this feature and in such cases I tend to study the code a lot (spend over an hour I guess 😮 )...

At the moment, I'm not quite satisfied with all the hardcoded color stuff in YAFCui/Core/Structs.cs and YAFCui/Rendering/RenderingUtils.cs. However, given the substantial effort required for refactoring, I refrained from doing so at this time.

Instead of hard-coding it won't be too hard to store them in a config file, so users can even make their own color schemes 😄 (but I agree that would be for another feature/PR)

I also tried it out and I have some issues with the dark theme:

1. The green works: barely visible when all is green, but next to None is it clear (at least on my screen, in the screenshot it is hard to read the text...). Same for red, it is subtle and I like it (on my screen) 😄
   But blue (in the image) hurts my eyes. Same for yellow.

2. When hovering icons on blue and yellow, the background color is (still) None which is quite ugly. (this can be mitigated by making the colors more subtle I guess)
   The better/best approach is to update the GUI to have colored lines and take the hover color of the line as the icon hover color. (somehow, probably enough changes for its own/new PR )

3. The grey disabled texts are not rendered anymore for lines with a color (I use times for example for 'future plans' when I am not 100% sure about yet: then I disable (so it does not impact) and mark it with the warning symbol as a reminder)

4. Analysis error symbols are not taken into consideration when coloring lines? I would expect it to be nice as it points out errors very clearly... Or maybe on the other hand it would be 'too much'? 🤔

[shihan42]

Sorry for the wall of text and the amount of comments. I do like this feature and in such cases I tend to study the code a lot (spend over an hour I guess 😮 )...

No need to be sorry. Quite the contrary, your thoughts are very helpful. Thanks!

Instead of hard-coding it won't be too hard to store them in a config file, so users can even make their own color schemes 😄 (but I agree that would be for another feature/PR)

Absolutely. But since a large part of the code base is sprinkled with usages of SchemeColor.xyz, it would be a big pile of work.

1. The green works: barely visible when all is green, but next to None is it clear (at least on my screen, in the screenshot it is hard to read the text...). Same for red, it is subtle and I like it (on my screen) 😄
   But blue (in the image) hurts my eyes. Same for yellow.
2. When hovering icons on blue and yellow, the background color is (still) None which is quite ugly. (this can be mitigated by making the colors more subtle I guess)
   The better/best approach is to update the GUI to have colored lines and take the hover color of the line as the icon hover color. (somehow, probably enough changes for its own/new PR )
3. The grey disabled texts are not rendered anymore for lines with a color (I use times for example for 'future plans' when I am not 100% sure about yet: then I disable (so it does not impact) and mark it with the warning symbol as a reminder)
4. Analysis error symbols are not taken into consideration when coloring lines? I would expect it to be nice as it points out errors very clearly... Or maybe on the other hand it would be 'too much'? 🤔

I've redone the dark theme (1). No bright and flashing colors anymore, but more subtle nuances. (2) got solved along the way, since it now looks ok.

(3) & (4) are somewhat debatable, though 😄

In the last days, I played around with the colored rows. For optical reasons, I like them more than the greyed text for disabled lines. The fact that marking a row overrides any other styles is something I like, personally. But I think it wouldn't be much of a hassle to implement another approach for rows that are disabled. Perhaps through the two color rows (background alt, text faint) that are unused now (0x0). However, to implement that effectively, I would need a deeper understanding of the coloring system, of which I currently have about... none 😄

I'd also say that (4) could easily be too much. But on the other hand... the rows with errors use the same symbol as the red row tag. It might be a good idea to use red highlighting to indicate errors in general. That would introduce possible confusion with "normally" red marked rows, but I don't think thats too much of an issue.

What do you think?

[veger]

I checked the new colors and indeed they are not hurting my eyes anymore 😄
But the contrast between the background and the text is too low:

The red background I can just read, but the other colors I cannot without squinting my eyes and and applying a lot of willpower. So I would keep the background colors, but tweak the text colors a little.

Edit: I just realized that I didn't notice the background issue when hovering, so that is 'fixed'! 😆

Maybe it is also nice to update the issue and ask @RandBlade on their opinion of the colors as they requested this?

The fact that marking a row overrides any other styles is something I like, personally.

There is no 'priority' defined for coloring styles... So any implementation would be good and then it boils down to personal preference. My only worry is that there is no visual distinctions anymore between a disabled colored row and a regular colored row. It will be apparent after a reload, as YAFC is not showing the production/consumption rates anymore. So maybe that would be a proper fix, to hide them directly when disabling...? (@have-fun-was-taken any opinion on this?)

the rows with errors use the same symbol as the red row tag. It might be a good idea to use red highlighting to indicate errors in general

Yes, so either use different symbols or treat them both as errors I suppose.
Using different symbols might be best, as errors and tags are different things. (Unless you tag a recipe as an error manually... lol 🤔 )

But now it is kind of confusing, as of some error icons (tags) you see a red line and for some others (errors) you don't...

[shpaass]

there is no visual distinctions anymore between a disabled colored row and a regular colored row

We want to prevent that for sure.

So maybe that would be a proper fix, to hide them directly when disabling...?

@veger, could you please elaborate on what you mean by "hiding" disabled rows?

[veger]

I meant hiding the production values (numbers/rates) of disabled rows. This is done when reloading the project (technically the values are not calculated when loading as they are disabled...)

[shpaass]

I meant hiding the production values (numbers/rates) of disabled rows. This is done when reloading the project (technically the values are not calculated when loading as they are disabled)

I think it would need to be handled in another issue and PR, unless @shihan42 thinks that this change can be achieved in minimal time and effort.

[shpaass]

Regarding the merge, @shihan42 feel free to merge when you consider the points to be sufficiently addressed.

[shihan42]

I'll look into it. Disabling a recipe is an entire different situation than simply tagging them. Hiding the numbers would even be better 👍

Regarding error tags and error markers: I'm not happy with it, either. Using the same symbols for errors and the red tag doesn't seem appropriate in the first place. So we could either remove that tag or introduce another form of coloring for real errors, e.g. more aggressive red hues. Opinions?

[veger]

I like the idea to have a more 'aggressive ' red for errors quite a bit. Then it still leaves the possibility to tag some recipe with the error symbol as well (which I often do).

[shihan42]

I like the idea to have a more 'aggressive ' red for errors quite a bit. Then it still leaves the possibility to tag some recipe with the error symbol as well (which I often do).

First draft:

What do you think?

[veger]

For me it looks nice. The 'aggressive' red immediately looks like an error to me. It has a high enough contrast with row highlighted red.

[shpaass]

@shihan42 feel free to merge when you're ready, although I'd recommend to squash the tweaks before that.