Disable animation between home and soldering screens if detailed view is set for both modes (probably a finally proper fix for #2076)

[ia]

• Please check if the PR fulfills these requirements

• The changes have been tested locally
• There are no breaking changes

• What kind of change does this PR introduce?
  Disable animation between home and soldering screens if detailed view is set for both modes.

• What is the current behavior?

  • go to settings and set detailed for idle and for soldering;
  • exit from settings;
  • try to switch from home screen to soldering mode back and forth;
  • result: double-slide effect which is considered as a minor bug.

• What is the new behavior (if this is a feature change)?

  • smooth non-animated transition between home & soldering.

• Other information:

  • Original bug report;
  • @discip comment clarifying original bug report.

[ia]

@discip, please, test this once you have time. I may be wrong but as far as I finally understand this is the exact fix that you were talking about.

[ia]

I tested this myself before making PR, but it seems it works and doesn't break anything else, hopefully.

[ia]

@Ralim, just for fun: at first, right after looking for enum values of OperatingMode, I was seriously thinking about something like newMode + context.previousMode == 1, but then I thought that it's better to have a long line, than the twisted brain. :D

[Ralim]

Heh, yeah those optimisations have bitten me in the past though. Let the compiler make them 🤣

[Ralim]

Thinking out loud here, would it be more logical to do these checks in the logic for the detailed mode files; and just have them pick which transitionMode to set on the context?

That way we keep all their logic grouped and don't have magic overrides?

[ia]

Thinking out loud here, would it be more logical to do these checks in the logic for the detailed mode files; and just have them pick which transitionMode to set on the context?

That way we keep all their logic grouped and don't have magic overrides?

I thought that since we are handling animation here to tweak the animation behavior, then one line of basic logic checks all together in one place as closer to the main animation logic routine as possible would be more clear, short & simple, than stuffing different checks to different parts of code. Like in case with detailed it would be four different places already (mono_96x16 + mono_128x32 x home + soldering) with extra duplicating of snippets. Are you sure that it will be better on a long run? Or am I missing something?

And considering the name of the directory with mentioned files - UI/drawing, and its content, it's the UI drawing routines for the specific modes/screens, but not the logic how transition is made & processed between those specific modes, as far as I understand.

Or probably I was confused by you mentioning detailed files...

[Ralim]

Ah sorry, you did what I intended, not what I said. Oops. You correctly figured what I meant, as in to put it in the logic for those views rather than a global override.

[ia]

I did confuse file names indeed, pardon me.

However, for the sake of argument, to illustrate my main point here: one line of code with checks (and comment) in one place VS alternative solution in the latest commit with multiple scattered lines of code across functions in logic/Soldering.cpp and logic/HomeScreen.cpp (briefly tested the build, the result seems the same).

Is this your suggestion? But why? We have such a perfect place in GUIThread.cpp with all information we need right there: current and previous modes, transitionMode effects, settings... the whole context! Are there any pitfalls just to check everything with one line there? I'm not being rude here, just really very curious now.

[Ralim]

You did what I meant so don't apologies.

Are there any pitfalls just to check everything with one line there? I'm not being rude here, just really very curious now.

Yeah this is a very classic question.

The idea I'm going for here to keep the logic and concerns separate.

In that, the main gui dispatch code should know only the absolute bare minimum about each screen and what its for. Mostly its job is to just handle calling the logic(), and get the stuff put on screen.

Where as the "special logic" that can change per screen I'm trying to encapsulate into the UI/logic folder. That way if your looking for anything that involves "the home screen" its really only one place to go looking.

This means I'm leaning more on the compiler to not be dumb and do a reasonable job minifying the generated code. But over time I've found its better at that, than people are at learning where all the code goes. So if you came to the code base having never seen it before, and you were looking for how the device acts when you do something on the home screen, I'm hoping that by putting it there its more obvious where to find it. Rather than finding code that contradicts what your seeing and you then have to start finding all references in the code base to find out where its overridden.

If that makes sense; always keen for feedback, especially as I know this is a tradeoff around potential-for-optimisation vs human-maintainability.

[discip]

@discip, please, test this once you have time.

Most likely, this will happen in the evening.

[ia]

@discip, please, test this once you have time.

Most likely, this will happen in the evening.

Please, take your time and let me know about the results, but don't merge it yet. Thanks.

[discip]

@ia
First of all: Thank you for your dedication and the effort you put into this.

Yeah, that’s exactly what I was hoping for back then.

After all this time I got used to the way it currently works! 😃
Also, I was not the one asking for a change, I just remembered when the other user mentioned this.

In my opinion, this is not really necessary.

However, feel free to do what you think is best. 😊

One more thought on that one:
Maybe other users even love the current behavior, since the sliding transition is effectively more noticeable.

[ia]

@discip, thanks for testing!

First of all: Thank you for your dedication and the effort you put into this.

Thanks. Much appreciated as always.

After all this time I got used to the way it currently works! 😃

Yeah, I know, it happens. :)

Also, I was not the one asking for a change, I just remembered when the other user mentioned this.

Yes-yes, I got it. But it was you who described the problem more clearly, than the original reporter.

In my opinion, this is not really necessary.

But I think you both were right! That double-effect of side-scroll between home/idle and soldering, when both detailed modes are enabled, is more like a little ux bug, than feature. I'm glad we all came to the compromise. P.S. I really like you, guys. :)

@Ralim, thank you as always for your very elaborative response explaining your perspective. I promise I will find some time to share my personal subjective opinion about the points you brought, but this weekend and the next week will be very busy for me, so I will be trying to keep up as harder as can, but probably very sporadically.

[Ralim]

@ia

Never a rush; and discussions are always welcome (you are welcome to tell me I'm wrong too 🙃 )

Thank you for taking this on:)