fix(out_base): update 3D variant aspect matching logic

[mdeweerd]

• Correct logic to select default 3D slot selection (added any_matched)
• Renamed default to default_slots for clarity.
• Add debug logging for variant matching
• Apply default 3D models only if no variant matches

[mdeweerd]

This is WIP. It already fixes some issues with the default selection.

However, I try to match when a Key is listed in the variant expression (abc,Key,def) to select a slot configuration.

For some reason "j8_clm" would match a variant not containing that key at all (matched = self.variant.matches_variant(var)).

logger.debug(f'XXX 3D: Test {var} vs {self.variant!r}: {matched!r}. Slots: {slots}')

produces:
                
DEBUG:XXX 3D: Test j8_clm vs '' (ph_cl_tbxy) [kibom]: True. Slots: ['3'] (kibot - out_base.py:761)

Digging a bit deeper.

[mdeweerd]

I figured it out, I had to prefix the variant key in the footprint text fields $+key:3$ for instance.

[mdeweerd]

@set-soft I did not find your review, I modified to have less changes.
I think technically there can be even less, but it's also important IMHO to differentiate the local match from the boolean indicating there was any successful match.

[set-soft]

Isn't this enough?

[mdeweerd]

I wrote "I think technically there can be even less" and explained why I prefer to keep the other changes as well.

[set-soft]

Once we have a match the loop ends, why we need two "match" variables?

[set-soft]

@set-soft I did not find your review,

My fault, reviews must be approved, they doesn't become visible until I confirm them.

I modified to have less changes. I think technically there can be even less, but it's also important IMHO to differentiate the local match from the boolean indicating there was any successful match.

But when we have a match the loop ends. Both if matched has a break. There is no need to have 2 variables.

[mdeweerd]

But when we have a match the loop ends. Both if matched has a break. There is no need to have 2 variables.

It helps with readability, code analysis and future changes (for instance checking/using or warning about multiple matches).

Anyway, updated.

[set-soft]

Thanks!

[set-soft]

@mdeweerd : the above patch removes the need of a flag and addresses your concern.

[mdeweerd]

@set-soft Thanks

In terms of code optimisation/DRY coding, we could have just one self.apply_list_of_3D_models(enable, slots, m, var) .

By setting it after the loop when slots is not None.

• Initialise var to 'default', slots to None.
• Set slots when 'default' is found, but continue the loop.
• Set slots when a variant match is found, set var, and exit the loop.
• After the loop, call apply_list_of_3D_models() when slots is not None.

[set-soft]

In terms of code optimisation/DRY coding, we could have just one self.apply_list_of_3D_model

Sure, the above patch does it, also moves the search to a function