My thoughts were:

1. You need to know a name of the variable first.
2. You need to know an interface of a class represented by the variable .

Does this work? I would have thought the definition of DeveloperOverlay was unknown at that time.

I guess it works because it's js and it's not parsed in a conventional way

It should: https://eslint.org/docs/rules/no-use-before-define. But I'm not sure do we need that?

I'd rather put it below the declaration, I think it's more consistent with what we do elsewhere and c++ code.

I agree. It will likely cause issues if/when we used ES6 classes after sm 45 is merged. Should be in CC

Like this:

DeveloperOverlay.prototype.commands = [
    // ...
];

?

yes
not sure if Commands or commands

By CC we have commands.

checked may be reused after init, for example if another player revealed the map (doing it once per turn sounds like accumulating performance cost, dunno)

Yeah, that was an idea for the separate layout function.

Might want to rename "callback" to "press" or "onPress" and "initValue" to "checked", so that the property names of this object match the property names of the GUI object, thus making it a bit easier for the reader.
...
might not be bad practice to use one property per line if there is more than one

It makes sense.

I got an error, this isn't the DeveloperOverlay. We create an array and only then assign it to the prototype. I didn't go deeper, but it seems logically.

Yes, self is used for capturing the current scope this.

Do you mean this:

DeveloperOverlay.prototype.commands = [
  // ...
  { "checked": self => self.controlAll },
];

?

If yes, it looks more like a workaround. Because the problem is that we try to use the external function like it's an object method. Probably we could use "checked": overlay => overlay.controlAll, but it still looks strange for me.

DeveloperOverlay.prototype.getCommands = () =>...; doesn't work too, it captures wrong scope.

No, @elexis's idea is to have commands as prototype.

Actually that was the first version (initializing in ctor), but @elexis`s suggested to separate it to have a shorter ctor.

Why? It's not DeveloperOverlay.

I'd suggest to remove it in a separate patch. Because there're at least 4 files that touch the g_DevSettings.

Indeed.

But the this.controlAll is set each turn from the simulation state. this.controlAll is used as a cache, instead of calling the GetSimState everytime.

Do you mean that the checked => g_Selection.SetMotionDebugOverlay(checked) returns a function that returns value of g_Selection.SetMotionDebugOverlay(checked)?

Nope, it's just an event.

Does Engine.GuiInterfaceCall("SetObstructionDebugOverlay", checked) return something then ? Else you should change all the lines, not only one.

Initially I thought to re-layout the overlay, but it's not needed.

You're right, I missed Engine.GameView_GetConstrainCameraEnabled there (as I wanted to add).

We see again how a review is important. Thank you!

Do you mean:

messageBox(
	500, 250,
	translate("Note: time warp mode is a developer option, and not intended for use over long periods of time. Using it incorrectly may cause the game to run out of memory or crash."),
	translate("Time warp mode"));

?

yes

Do we have some basic structure of such object? JS doesn't have a classic OOP. So it's harder to avoid some global/public stuff.

Should the object be like this?

function DeveloperOverlay() {} // Or any other way to define an object.

DeveloperOverlay.prototype.layout = function() {
    // ...
}

let overlay = new DeveloperOverlay();

P.S. Some time ago wraitii sent a link to a video about OOP, it's called like "OOP is dead, Data Driven/Oriented Design is a key" :P

That's what I had in mind, DeveloperOverlayManager and g_DeveloperOverlayManager maybe.
I guess one can do it without prototype as well, since there will be only one instance of this object, but it might be cleaner with prototype.
I guess data-oriented design is mostly relevant for entity data.
I didn't check how to compartmentalize the related code.

Usually *Manager means that it manages something (not itself), for ex. multiple objects.

It's not present.

onPress looks ok for me.

Yeah, I meant I agree with the onPress modification.

Already fixed.