Code review comment for lp:~alan-griffiths/compiz-core/init-sequence-fix

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

> Sam, I didn't want to change the runtime behaviour in the way you suggest.
>
> The result is a workaround true, but the prior workaround was to test a flag
> inside the addAction member function (which was already in "undefined
> behaviour" land).

So on the count of "its undefined behaviour", I'd be happy to merge this anyways.

>
> There are several issues that could be addressed in a larger fix:
>
> 1. I'd prefer to avoid the need for tests by ensuring the initialisation
> happens in a sane sequence (i.e. "screen" is intialised before CoreOptions is
> constructed).
>
> 2. I don't like having the global "screen" variable at all - objects that need
> a pointer to the CompScreen object should have it supplied on construction.

This is probably a lot more feasible than it looks - all plugins are already passed a CompScreen on construction (DI), it is trivial for them to use that. As for the other objects - all windows can store a pointer to the CompScreen object and then the other stuff is also easy, although we really, for things like actions we should just be passing them interfaces for ActionStorage or whatever and the implementation is CompScreen (for now).

« Back to merge proposal