Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/672/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2087/display/redirect

WithReturn can be avoided from the name, the functions are distinguished by signature already and do the same except return value.

This can be used to get a feedback if the function called through ScriptEvent handled the event.

It sounds like boolean would restrict the use cases of that function then, that the caller that expects a boolean can do that after calling the function and that other callers might consume something other than boolean.

I dont know the use case envisioned but the use case seems conceivable in gernal, so diff direction seems correct.

In D2727#115077, @elexis wrote:

WithReturn can be avoided from the name, the functions are distinguished by signature already and do the same except return value.

Why? They have the same signature and only distinguish in the return value. Some parts of the gui need ScriptEvent to have no return value. (e.g. RecurseObject)

This can be used to get a feedback if the function called through ScriptEvent handled the event.

It sounds like boolean would restrict the use cases of that function then, that the caller that expects a boolean can do that after calling the function and that other callers might consume something other than boolean.

So you propose just returning a JSValue?

I dont know the use case envisioned but the use case seems conceivable in gernal, so diff direction seems correct.

I already teased the use case in the diffs description: the bool will say whether this event has been handled or not.
I can't think of use cases, that use a nonboolean return value.

In D2727#115103, @Imarok wrote:

In D2727#115077, @elexis wrote:

WithReturn can be avoided from the name, the functions are distinguished by signature already and do the same except return value.

Why? They have the same signature and only distinguish in the return value. Some parts of the gui need ScriptEvent to have no return value. (e.g. RecurseObject)

Is there a compile error if RecurseObject gets a function with ignorable return value instead of a void?
Aside from that one can still name the void and function the same way if both signatures are necessary and the compiler will chose the signature it needs.

This can be used to get a feedback if the function called through ScriptEvent handled the event.

It sounds like boolean would restrict the use cases of that function then, that the caller that expects a boolean can do that after calling the function and that other callers might consume something other than boolean.

So you propose just returning a JSValue?

I dont know the use case envisioned but the use case seems conceivable in gernal, so diff direction seems correct.

I already teased the use case in the diffs description: the bool will say whether this event has been handled or not.
I can't think of use cases, that use a nonboolean return value.

Just because we can't conceive use cases of non-boolean return values doesnt mean that its best to cast the return value to boolean in any case - the alternative is to let the caller decide, if a caller wants bools, it can cast to bool, if a caller wants a function, string or numbers as return value, it can use it, but not if it only provides a boolean. So the only reason to cast tobool in that function would be if there would be many callers that all want booleans (i.e. too much code duplication), or if returning the JS value would cost more performance, but as far as I know it only returns a pointer that already exists.

I.e. cast to bool inside the function would be useful if it improves performance or avoids 20+ bool conversions outside of the caller, otherwise the function becomes more versatile if its the caller that casts to bool.

In D2727#115107, @elexis wrote:

In D2727#115103, @Imarok wrote:

In D2727#115077, @elexis wrote:

WithReturn can be avoided from the name, the functions are distinguished by signature already and do the same except return value.

Why? They have the same signature and only distinguish in the return value. Some parts of the gui need ScriptEvent to have no return value. (e.g. RecurseObject)

Is there a compile error if RecurseObject gets a function with ignorable return value instead of a void?
Aside from that one can still name the void and function the same way if both signatures are necessary and the compiler will chose the signature it needs.

No, you can't overload a function only by changing the return type.

This can be used to get a feedback if the function called through ScriptEvent handled the event.

It sounds like boolean would restrict the use cases of that function then, that the caller that expects a boolean can do that after calling the function and that other callers might consume something other than boolean.

So you propose just returning a JSValue?

I dont know the use case envisioned but the use case seems conceivable in gernal, so diff direction seems correct.

I already teased the use case in the diffs description: the bool will say whether this event has been handled or not.
I can't think of use cases, that use a nonboolean return value.

Just because we can't conceive use cases of non-boolean return values doesnt mean that its best to cast the return value to boolean in any case - the alternative is to let the caller decide, if a caller wants bools, it can cast to bool, if a caller wants a function, string or numbers as return value, it can use it, but not if it only provides a boolean. So the only reason to cast tobool in that function would be if there would be many callers that all want booleans (i.e. too much code duplication), or if returning the JS value would cost more performance, but as far as I know it only returns a pointer that already exists.

I.e. cast to bool inside the function would be useful if it improves performance or avoids 20+ bool conversions outside of the caller, otherwise the function becomes more versatile if its the caller that casts to bool.

I think it's good to provide an easy clean interface. And I see many usecases for returning a bool and not many for returning any other type.
If we do the conversion already in the function, nobody else has to fiddle around with JS and the conversion thingy.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/gui/ObjectBases/IGUIObject.h
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/gui/ObjectBases/IGUIObject.h
|  53| class·IGUIObject
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classIGUIObject{' is invalid C code. Use --std or --language to configure the language.

source/gui/ObjectBases/IGUIObject.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2089/display/redirect

Disclaimer ahead - that I dont click on request changes means I don't request changes. I am only commenting, if the argument is convincing it can be followed, and it can be disregarded if the argument is not convincing.

In D2727#115110, @Imarok wrote:

No, you can't overload a function only by changing the return type.

Ah you,re right, and the argument for that is that the compiler wouldnt know which one to chose if there are two that have the same name and take the same arguments but dont use the return value.
It might be that only one function (the one with original return value from the JS function called) is sufficient for all cases.

So you propose just returning a JSValue?

I would have to look at the specs again whether it was JS::Value, but I think that was it and that it didn't have a performance impact if not rooted again.

I.e. cast to bool inside the function would be useful if it improves performance or avoids 20+ bool conversions outside of the caller, otherwise the function becomes more versatile if its the caller that casts to bool.

I see many usecases for returning a bool and not many for returning any other type.

I suspect there would be 3 or 4 function calls, so it might result in 3-4 more lines of code while removing 3-4 lines of code elsewhere to gain the benefit of obtaining arbitrary return values, could that be?
Not many or none? If there was one use case for non-bool return value, one use case for no return value, one use case for bool return value, why have three functions instead of one for about the same amount of code in the caller.
The argument is to maximize versatility of the GUI with least amount of code.
You might be right as I didnt apply the diff locally to measure how many lines of code the boolean conversion after the function call would make.

If we do the conversion already in the function, nobody else has to fiddle around with JS and the conversion thingy.

Would it be more than one ToBoolean call?

I think it's good to provide an easy clean interface

I agree! But I dont see why being more restrictive is cleaner. JS::Value is the original return value and appears throughout the GUI code.
Perhaps there are only boolean use cases, I suspect there to be a non-boolean one, perhaps we can construct one with some imagination.

I didn't understand how this function will be used exactly, but if CMinimap.cpp will consume a boolean, is it conceivable that it will consume a number or string (for example the type of the preselected action), or another GUI object type consuming another object?

In D2727#115118, @elexis wrote:

I didn't understand how this function will be used exactly, but if CMinimap.cpp will consume a boolean, is it conceivable that it will consume a number or string (for example the type of the preselected action), or another GUI object type consuming another object?

No.
I kind of explained already why I need this diff. But I hope my next diff makes it clearer.