<div dir="ltr">Hi, Stefano!<br><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jul 1, 2015 at 7:38 PM, Stefano Bonicatti <span dir="ltr"><<a href="mailto:smjert@gmail.com" target="_blank">smjert@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div>The idea to solve this is first of all to have them be only widget, so they take input and display something, but they should have the less logic possible.</div><div>This means having a central class that deals with presets, this class contains the current selected preset that must be in sync with the other widgets.</div></div></blockquote><div><br></div><div>Technically speaking we have one. It is KisCanvasResourceProvider (aka KoCanvasResourceManager). But its main problem is that when it emits a "preset-changed" signal we cannot distinguish who has initiated one. We also have KisAcyclicSignalConnector class. I'm not sure it helps you here, but the idea is the same. It breaks loops in the updating process.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>This has the ultimate decision of which preset is the current one.</div></div></blockquote><div><br></div><div> Well, KisCanvasResourceProvider is the one.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Widgets will only speak with this class about selected resources so that the class know where the events come from and know when to ignore or send an update to the other widgets.</div><div>For this to work the widgets can give updates to this class through signals but the class itself will directly call widgets functions, not through signals.</div><div>This means a widget will register to this class and thanks to a common interface the class will be able to speak with the widget.</div><div>This is to avoid having the class sending signals that are caught by every widget, when it should've been caught by only 2, where then the third has to know, somehow, that it should discard that event.</div></div></blockquote><div><br></div><div>Really looks like what KisAcyclicSignalConnector was created for.<br></div><div dir="ltr"> <div><br></div><div><br></div><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">TL;DR:<br>1) We have issues with widgets presets being too smart and fighting each other when selecting a preset, especially between the preset docker, preset popup and the preset strip.<br>2) We don't distinguish call paths that comes from or signals that are emitted due to UI or non UI events.This leads to problems because in some cases creates loops (that are dealt with but in a way that creates other functionality issues).We should split these paths by using different functions and different signals and we should be sure that one can't trigger the other.<br>3) We have generic signals (canvasResourceChanged) that are used to keep track of specific things (presets), but we don't check if this specific thing is really changed and we do unnecessary work that might also create functionality issues.<br>4) We have reuse of signals beyond scope, like resourceRemoved to force an update of the resource model, but this triggers code that it shouldn't be called in some cases. We need to have specific signals for specific operations.<br>5) We have fighting occurring between higher classes and base classes too, about the resource selection (preset widgets fighting with KoResourceItemChooser etc).We probably should have base classes that are more dumb than we have now.<br>Solution: a part from the ones given in the list, having a central class that deals with keeping track of the current preset and talking with the widgets avoiding loops or avoiding to call widgets function due to signal sent by those same widgets, i think, solves this issues.Widgets should only take input and display results, they will only be able to directly affect this central class through signals and only very indirectly and with the necessary controls, the other widgets.<br>This is already quite long so i will end this here, but is necessary to understand the need of the refactoring and also the fact that it can't really be done in steps.. because there are a lot of places that are failing and they are all connected so we need to fix all those together.Next step from me is to obtain a sort of graph of interactions to have in front of the eyes what currently happens, between all the classes involved, when a preset is selected, or what happens when something generate a preset selection.</blockquote><div><br></div><div>Well, central class idea is fine, except that we already have a central class, KisCanavsResourceProvider. Though you can add one class below it, yes. <br><br></div><div>I guess that the refactoring can be done in kind of "steps". You need to define a common interface for such widget objects. And probably consider using KisAcyclicSignalConnector. The latter one will allow you to avoid special hardcoded function calls breaking the loops. After you defined a new interface, you can wrap in into a special object, which is technically mediator [0] for KisCanavsResourceProvider. After you have your mediator implemented you can clean the usages of old signals in the widgets one by one.<br></div></div><br><br>[0] - <a href="https://en.wikipedia.org/wiki/Mediator_pattern">https://en.wikipedia.org/wiki/Mediator_pattern</a><br><br></div>-- <br><div class="gmail_signature">Dmitry Kazakov</div>
</div></div></div>