HaikuArchives / ffmpegGUI

GUI for FFmpeg
MIT License
23 stars 10 forks source link

Spinners: Allow not doing an Invoke() on SetValue() #114

Closed humdingerb closed 1 year ago

humdingerb commented 1 year ago

We want to be able to do a SetValue() without there being an Invoke() triggered, which sends a message as if the user used the spinner. No other BControl derived class does this. This is a stop-gap until Haiku's BSpinner class is fixed, see [1].

Now, we have an optional bool parameter: SetValue(in32 value, bool invoke = true); To avoid triggering Invoke(), set the bool to false.

[1] https://dev.haiku-os.org/ticket/18305

andimachovec commented 1 year ago

I thought that overridden virtual functions always must have exactly the same parameters as in the base class. My C++ book seems to confirm this. That leaves me surprised that g++ doesn't give us an error or at least issues a warning. @pulkomandy or @waddlesplash , could you maybe give us some insight on this matter?

waddlesplash commented 1 year ago

Warnings don't occur because it will simply implicitly create an entirely new virtual function.

C++11 introduced the override keyword to mark methods that are intended to override ones in the base class, which will error out when no such method exists, to compensate for this.

andimachovec commented 1 year ago

Thanks for the explanation, @waddlesplash :+1:

andimachovec commented 1 year ago

@humdingerb : So you could add a new function that sets the bool value for Invoke() instead of overriding SetValue().

humdingerb commented 1 year ago

Alright. See if I understood correctly. Function name OK? Note, we don't use SetWithoutInvoke() yet, we'll only need it when editing a job (for now), once that PR gets merged.

andimachovec commented 1 year ago

Yep, looks good 👍

pulkomandy commented 1 year ago

I thought that overridden virtual functions always must have exactly the same parameters as in the base class

Yes, that's the definition of override. When the parameters don't match, it's called an overload instead, and doesn't replace the other function

humdingerb commented 1 year ago

Alright. Thanks all around.