Attribute fallback is faulty

[egmontkob]

Attribute fallback is briefly explained in sand256.ini.

(It forgets to mention that after falling back to the given section's _default_ there's in turn another round of fallback to [core]/_default_.)

Accordingly, in a scenario like

[core]
    _default_ = black;green;italic
    selected = black;red;bold

in mc the selected file is bold, but not italic.

In the skin viewer the selected file is both bold and italic.

---

There are some other tricky cases, like the keyword "base" documented there, but not used in any of the skins (maybe we can remove that feature?), I haven't checked how the skin viewer handles that.

While working on #11, I noticed that the current design relies on CSS falling back to the container element's class, as there are nested spans, e.g. <span class="dialog_dhotnormal"> is always nested under a <span class="dialog__default_"> and if this nesting is manually removed then dhotnormal's colors don't appear correctly.

Automatically emitting such nested spans would be somewhat cumbersome. Instead, I'm currently emitting unnested spans with multiple classes, i.e. <span class="dialog__default_ dialog_dhotnormal"> which results in the same CSS fallback. However, since mc's fallback mechanism cannot directly be mapped to CSS, I think I'd prefer a solution where the JS code performs the fallback logic and generates a CSS that contains all the desired attributes for each and every class (without the need for fallback in CSS), so I could just emit an unnested <span class="dialog_dhotnormal"> and it would work.

[zyv]

There are some other tricky cases, like the keyword "base" documented there, but not used in any of the skins (maybe we can remove that feature?), I haven't checked how the skin viewer handles that.

Maybe @mc-worker can say something about that and/or say whether he's okay with removing it.

Automatically emitting such nested spans would be somewhat cumbersome.

Is it (honest question)? My feeling is that dealing with it in JS would be even more cumbersome. Besides, I think that an argument can be made that this kind of logic ideally belongs to the generator and not in the JS visualizer app.

[egmontkob]

Is it (honest question)?

Nested spans vs. multiple classes results in the same html/css behavior, and I found multiple classes to be easier to emit.

I'm not sure if we can rely on css's fallback behavior because it is a hardcoded behavior with no flexibility, and we'd somehow need to trick it to work exactly like we want it to. E.g. handling that "base" keyword. Or the attribute thing.

We do need to have some JS, we need to parse the skin file, handle each component, know what it means if the attributes are missing, know what it means if an attribute is present (which is mishandled now), and we need to perform these steps whenever the user edits the skin. So parts of this will have to remain in JS. As much as I'd prefer not to touch JS too much, I'd still find it preferably to have the logic at one place, rather than split across multiple components.

Now, maybe this particular issue of fallbacks can be addressed with keeping the current split architecture. If there's any attribute, e.g. bold, (or any invalid word, e.g. "none") then not only would we need to enable the attributes listed there but also disable the attributes not listed. And leave the fallback to CSS classes. This could work, too (although cannot handle the keyword "base"). I think I have a slight preference for the end result where skin parsing and the fallback logic is handled at the same place (which would be JS), even if this might be a tiny little bit more work at an area we're less experienced.

Whoever will get to work on it will see which one turns out to be simpler / cleaner. We're not looking for clear design here but for practicalilty.

[egmontkob]

JS-level fallback will be needed if we want to support the "reverse" attribute (currently not supported by the skin viewer/editor).

[egmontkob]

Doing the fallback entirely in JS, and not at all by applying multiple classes, wasn't a perfect idea.

It falls short when a (possibly newly introduced) keyword is missing from the skin file. That's because the CSS is generated solely from the skin file, and thus those UI elements would have no CSS generated from them.

One possibility would be to parse the "screenshot" templates and catch all the classes used there, or to manually maintain a list of all the supported keywords. This way the CSS would always be complete, even if some color definitions are missing from the skin file. These don't sound the simplest to implement and maintain, so I decided not to go down this route.

The other possibility is to keep the nested or multiple classes, in the order of mc's fallback mechanism. E.g. a <span class="core__default_ dialog__default_ dialog_dframe"> would still apply the dialog's main colors to the frame even if the new dframe color isn't defined.

---

That being said, I think it increases robustness and debuggability, and helps implement the reverse attribute if the JS code also implements the same fallback logic. That is, emits as complete CSS definitions for each class as possible; relying on CSS's fallback mechanism only in case a keyword is missing from the skin file.

[egmontkob]

Fixed via PR #22.