These two functions beforeGUI/afterGUI always expect a return true/false (because of c++ I think)

GUIManager.cpp

		if (top()->GetScriptInterface()->CallFunction(global, "handleInputAfterGui", handled, *ev))

That is defined in the lovely NativeWrapperDefns.h.

So bool handled is the return value.

The return value is captured as a JS::Value in jsRet and then converted to the specified type in FromJSVal.

FromJSVal<bool> is defined in ScriptConversions.cpp and it contains WARN_IF_NOT(v.isBoolean(), v);. I guess that's the reason why we need a bool?

(I don't recall the JS Interface being so picky)

Anything is possible but why?

Shorter formulas can be easier to understand, here it's just equal.
It's easy to dismiss minimization for one or two statements, but excess code does accumulate, often one could remove more than 1/3 to 2/3 of the code. That are several thousand lines code.

if (X)
	return false;
return Y;

<=>

return X ? false : Y;

<=>

return !X ? Y : false;

<=>

return !X && Y;

<=>

return !g_InputEvents.isStateAfterGUI() && g_InputEvents.ProcessMessage({ "type": ev.type, "ev": ev });

So handleInputBeforeGui is truthy if !isStateAfterGUI && ProcessMessage, exactly like the functionname suggests. The reader doesn't have to infer this anymore, but can read it directly.

Authors have the choice to leave code as concise, simple and clean as possible, and thus always try to resolve the equations, or they consciously chose to not even try but keep what works. That's the path to the dark side, spaghetti mess, and creating new works by copying and modifying previous works.

Idk from where that space came from.

Sounds like the editor

I leave this for another patch.
Maybe all the var -> let change should too be another patch.

The original were undefined too so I must follow as I only change them to a vector2d format

Nice

Indeed

I suppose you are right. But the benefit of this built in method is that it checks if those states really exist (with warnIfStateNotDefined) so in the case someone wrote "PLAICNG" instead of the correct "PLACING" it will give a warning and a stack trace for better debugging.

Whether or not it warns is independent of whether the function supports arrays

hasBaseState prototype provides with the desired solution (being able to compare against multiple states or only just one) and makes guarantees that the comparison is between states and not some random strings.

• Because it doesn't affect the flow of the FSM (change of state )
• What other code ?

The function is called from L215.
The function could be defined in L118 or L460, but it's at L944, why? Keeping related code grouped minimizes fragmentation.

Because it doesn't affect the flow of the FSM (change of state )

Usually one moves code to a new function to break a long function into smaller parts, so as to make them easier to understand separately.
These are 20 lines, so maybe considerable, but the function that calls it now only has one line, so nothing really gained?
The function is also called only from this one location (a hypothetical second caller can still call the function through foo.bar.baz.hotkey()).

If it's really only the absence of an FSM state that favors moving it, there are some other functions that also don't change the FSM state.
One can also split the other functions that do change the FSM state to helper functions that are called, changing the FSM state depending on the return value of the helper function.