Added ioc context to inject. Step 5 #22808
Conversation
igorkorsukov
force-pushed
the
app/multiwin_step5
branch
from
eb6e734
to
64b3fa5
Compare
igorkorsukov
force-pushed
the
app/multiwin_step5
branch
from
64b3fa5
to
d827495
Compare
igorkorsukov
force-pushed
the
app/multiwin_step5
branch
from
d827495
to
51bc106
Compare
|
Perhaps for "stateless" things, like ProjectCreator, and Renderer, we could perhaps use one global instance, instead of one for each context. But we can do that in the future; I understand that it might be better to apply a uniform approach right now, and look later which specific cases can be refined. |
There was a problem hiding this comment.
What's the situation with these changes?
There was a problem hiding this comment.
I don't understand what to do with them (
They fail for me both locally and in СI.
I took the master, and these tests also fail, both with and without install.
Previously, they always failed for me locally, but they worked on СI..
Maybe you have some ideas and knowledge about these tests?
There was a problem hiding this comment.
I would need to investigate that. I don't recall having them seen failing yet , but I haven't run them locally for a while. Is there perhaps a log of a CI run where I can see how they fail?
There was a problem hiding this comment.
There was a problem hiding this comment.
What do you think about merge now as is (with commented tests), but later I fix it separated?
There was a problem hiding this comment.
Let's do that, I don't really have ideas right now of what could be it. Perhaps make tech debt issue for it, so that we don't forget.
| @@ -101,7 +101,7 @@ Ret DrawDataGenerator::processFile(const muse::io::path_t& scoreFile, const muse | |||
|
|
|||
| DrawDataPtr DrawDataGenerator::genDrawData(const muse::io::path_t& scorePath, const GenOpt& opt) const | |||
| { | |||
| MasterScore* score = compat::ScoreAccess::createMasterScoreWithBaseStyle(); | |||
| MasterScore* score = compat::ScoreAccess::createMasterScoreWithBaseStyle(nullptr); | |||
There was a problem hiding this comment.
What is the consequence of passing nullptr here? Is it that this score will not be able to find EngravingConfiguration and Renderer? (I guess that's fine (until the moment that it isn't fine anymore because it causes crashes), but just wanted to check)
There was a problem hiding this comment.
Now it is done so that if a ioc context is null, then the global ioc is used. It might be worth doing this more explicitly, for example passing "global ioc context"
There was a problem hiding this comment.
But not all interfaces are registered to the global ioc context, right? So those that aren't registered there, those will not be found here.
There was a problem hiding this comment.
All interfaces are registered in the global ioc, because the “global ioc” will simply be the first ioc (of the first window). Now this is not entirely true, but it will be so :)
| @@ -175,6 +175,16 @@ EngravingItemList EngravingItem::childrenItems(bool all) const | |||
| return list; | |||
| } | |||
|
|
|||
| const std::shared_ptr<IEngravingConfiguration>& EngravingItem::configuration() const | |||
| { | |||
| return score()->configuration.get(); | |||
There was a problem hiding this comment.
Is it right that the purpose here is to prevent making EngravingItem/EngravingObject Injectable, and only Score? (makes sense, just checking if I understand correctly)
There was a problem hiding this comment.
That's right!! Otherwise, we will have to pass a ioc context to each object... Although we are already passing the parent, and ultimately has access to a score.
Yes, I thought about this too and came to the same conclusions. |
No description provided.