D14688: [libkwineffects] Expose getting/setting activeFullScript to scripted effects

zzag accepted this revision.
zzag added a comment.


  In general, I'm okay with it.
  
  Please bump API version in kwineffects.h

INLINE COMMENTS

> effects.cpp:626
>      }
> +    bool activeChanged = (e == nullptr || fullscreen_effect == nullptr);
>      fullscreen_effect = e;

const bool

> effects.cpp:641
> +{
> +    return fullscreen_effect;
> +}

Readability nitpick: fullscreen_effect != nullptr

> scriptedeffect.h:41
>      Q_ENUMS(EasingCurve)
> +    Q_ENUMS(ActiveFullScreenEffectState)
> +    // True if we are the active fullscreen effect

Please delete this one.

> scriptedeffect.h:42
> +    Q_ENUMS(ActiveFullScreenEffectState)
> +    // True if we are the active fullscreen effect
> +    Q_PROPERTY(bool isActiveFullScreenEffect READ isActiveFullScreenEffect NOTIFY isActiveFullScreenEffectChanged)

/**
   *
   **/

> scriptedeffect.h:97
>  
> +    bool isActiveFullScreenEffect() const;
> +

It would be great to add @since but other fields don't have that...

> scriptedeffect.h:116
>      void animationEnded(KWin::EffectWindow *w, quint64 animationId);
> +    void fullScreenEffectActiveChanged();
> +    void isActiveFullScreenEffectChanged();

Is it emitted anymore?

> scriptedeffect.h:137
>      QHash<int, QAction*> m_touchScreenEdgeCallbacks;
> +    Effect* m_activeFullScreenEffect = nullptr;
>  };

Please put whitespace before `*`.

REPOSITORY
  R108 KWin

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D14688

To: davidedmundson, #kwin, zzag
Cc: zzag, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart