Connection to TDM with automation

[stgatilov]

On 12/29/2020 at 8:41 PM, peter_spy said:

That's still confusing. User doesn't have to be familiar with underlying implementation, hot reload is continuous sync of what's being done with the map in dr, right? So how 'one time hot reload' differs from save map and reload map in TDM, and how's that useful?

Regarding "reload map" vs "update map".

I guess "reload map" mode can be removed from GUI, leaving only the "update map"... if "update map" works well enough and nobody sees any reason to retain the other mode. For debugging purposes, I can always save map in DR and execute "reloadMap" in TDM console.

As for "always sync" vs "sync now" in "update map" mode, I think both are useful.

[peter_spy]

1 hour ago, kingsal said:

Tweaking textures! This is hands down the most tedious thing. Having to dmap and reload just to see texture changes is very time consuming.

What do you mean by that? Aligning textures on brushes? Otherwise, you don't need to dmap and reload the map, if you update texture images, models, or material declarations.

[kingsal]

29 minutes ago, peter_spy said:

What do you mean by that? Aligning textures on brushes? Otherwise, you don't need to dmap and reload the map, if you update texture images, models, or material declarations.

Tweaking texture in DR.

[stgatilov]

38 minutes ago, peter_spy said:

What do you mean by that? Aligning textures on brushes?

I guess @kingsal means changing textures on brushes.
Right now hot reload cannot do it, so dmap + restart is needed.

To be honest, tweaking monsterclip is a way much harder task among these two...

[kingsal]

1 hour ago, stgatilov said:

Regarding "reload map" vs "update map".

I guess "reload map" mode can be removed from GUI, leaving only the "update map"... if "update map" works well enough and nobody sees any reason to retain the other mode. For debugging purposes, I can always save map in DR and execute "reloadMap" in TDM console.

As for "always sync" vs "sync now" in "update map" mode, I think both are useful.

I can see cases where you might want RELOAD MAP. This is more just a convenience thing. Lets say I have a scripted sequence in my mission that only runs once that I am tweaking (something that doesnt work on update map). I can quickly save my map, hit the "reload map" and when I switch back to TDM, bam, the map has already started the reload process. This really is just saving a couple clicks but I think its worth it.

[kingsal]

3 minutes ago, stgatilov said:

I guess @kingsal means changing textures on brushes.
Right now hot reload cannot do it, so dmap + restart is needed.

To be honest, tweaking monsterclip is a way much harder task among these two...

Yep, changing textures in DR. I would argue that yes monster clipping is incredibly tedious, but I do that less often than I texture. 

[stgatilov]

29 minutes ago, kingsal said:

I can see cases where you might want RELOAD MAP. This is more just a convenience thing. Lets say I have a scripted sequence in my mission that only runs once that I am tweaking (something that doesnt work on update map).

If it runs once on trigger, you can probably respawn the trigger, and run it again.

[kingsal]

@stgatilov @OrbWeaver @greebo Just a heads up, as I am working I am finding RESPAWN SELECTED ENTITY very useful. It seems on update / reload certain things like hide_distance and _color on func_emitters do not update, so I find myself respawning those manually. 

 

also @stgatilovIt seems TDM doesnt set noclip, god, ect on camera sync when you restart the map completely from TDM. I've killed myself a couple time after dmapping and then using the camera sync. My thinking is that whenever camera sync is active, even after reload these cheats should kick back on. (Maybe we can make this a settings option?)

[stgatilov]

5 hours ago, kingsal said:

@stgatilov @OrbWeaver @greebo Just a heads up, as I am working I am finding RESPAWN SELECTED ENTITY very useful. It seems on update / reload certain things like hide_distance and _color on func_emitters do not update, so I find myself respawning those manually.

Yes, respawn should serve as a way to "force" full update.

Could you please report precisely cases which don't work, so that I could probably adjust builtin rules in future?

Quote

also @stgatilovIt seems TDM doesnt set noclip, god, ect on camera sync when you restart the map completely from TDM. I've killed myself a couple time after dmapping and then using the camera sync. My thinking is that whenever camera sync is active, even after reload these cheats should kick back on. (Maybe we can make this a settings option?)

It is pretty hard to understand when something is changed from within TDM. For example, in order to set god mode ON, "god" command is executed, then the console message is read, and if it says "god mode OFF", then "god" command is executed again.
There is no direct way to learn if god mode is ON, and although it is possible to add a specific automation command for this, I'd better avoid it. And it is even harder to learn when god mode changes: I believe current architecture is like HTTP, i.e. TDM cannot send stuff when it wants to, it only responds to requests sent from DR.

I think it would be better to add a way to "restart FM", and make it disable/enable camera sync automatically.

[stgatilov]

I think we can add a GUI button which would start or restart map in TDM. It can be used for many different features already proposed. I think there should also be a "dmap" checkbox near the button.

I see it working the following way:

1. If TDM is already connected, then execute "getviewpos" to learn current coordinates and disconnect.
2. Save the map in DR if it has local modifications.
3. If there is no TDM with automation ON, then start new one.
4. Connect to TDM instance with automation ON.
5. Select the current FM(project) if it does not match yet (extend automation: learn the current FM).
6. Run dmap if the "dmap" checkbox is enabled. If there is an error, then stop completely.
7. Start the map.
8. If camera sync was enabled initially, then enable it again (turning noclip/god/notarget ON).
9. If the player position was saved, then "setviewpos" to it.
10. If various other modes (i.e. map update) were on, then enable them back too.

I think it should cover the features like "start TDM automatically", "restart map", and "dmap map".

 

P.S. @OrbWeaver, I guess we need to search for yet another library, this time similar to Python's subprocess. I have written various process-starting C++ code with WinAPI in the past, and don't want to go through it again

[OrbWeaver]

7 hours ago, stgatilov said:

P.S. @OrbWeaver, I guess we need to search for yet another library, this time similar to Python's subprocess. I have written various process-starting C++ code with WinAPI in the past, and don't want to go through it again

Boost.Process can handle this, however we don't currently have Boost in DarkRadiant (since all of the Boost features we were originally using are now part of Standard C++, and I understand the Boost dependency complicates the build on Windows).

I see that wxWidgets has some process related classes, would these be sufficient?

[kingsal]

12 hours ago, stgatilov said:

I think we can add a GUI button which would start or restart map in TDM. It can be used for many different features already proposed. I think there should also be a "dmap" checkbox near the button.

I love this proposal if its feasible!

[kingsal]

14 hours ago, stgatilov said:

Could you please report precisely cases which don't work, so that I could probably adjust builtin rules in future?

Right so far this is what doesn't seem to be hotReloading correctly and requires respawning. I'll report more as I find them:

• Speakers are not updating "s_shader" and playing the newly selected sound.
• Func_emitters are not updating "_color" on particles. 
• Func_statics are not updating "hide_distance"

I've also noticed that occasionally the system isn't deleting the old entity. So in some cases its spawning a duplicate func_static or speaker when respawning the entity.

[stgatilov]

On 1/2/2021 at 2:59 AM, kingsal said:

I've also noticed that occasionally the system isn't deleting the old entity. So in some cases its spawning a duplicate func_static or speaker when respawning the entity.

Very unlikely.

But when you delete/respawn entity, its sound remains (that's default behavior when deleting entity). I'll make sure sound is stopped when entity is deleted via hot reload.

Quote

Speakers are not updating "s_shader" and playing the newly selected sound.

I'll force respawn if any spawnarg starting form "s_" has changed. Along with sound stopping fix, it solves the problem.

The harder problem is with lights. They have some stuff with "lit", "extinguished", "unlit", etc. suffixes, and script code which switches between them. I'm not yet certain how to deal with them.

[kingsal]

3 hours ago, stgatilov said:

Very unlikely.

This for sure happened. I'll capture a video or screenshots next time I'm in the game.

[kingsal]

@stgatilov EDIT : Looking into this further, I think this is a problem specific to these models. I can't seem to reproduce with other models. Might be an issue with the mesh or something weird?

This is what I did to get the duplicates (video below):

• Selected a group of func_statics. Reduced the current hide_distance and hit enter to apply to all:
• Hit respawn selected entities
• There are now duplicates spawned in the game.

Video here:

https://www.dropbox.com/s/722cns51h3wbedp/The Dark Mod 2.09_64 2021-01-06 10-55-39_Trim.mp4?dl=0

[stgatilov]

After looking at rather weak and varied response about game connection, I decided to put all available features into GUI.

Here is how it looks on FormBuilder:

1. Buttons should have better images
2. The "connected" checkbox is just a boolean indicator, I did not find any well-established widget for this.
3. I prefer adding lengthy tooltips over writing lengthy text labels.

The buttons on right side (from top to bottom): set camera from game, reload entities now, reload from .map now.
The buttons on bottom (from left to right): respawn selected entities, pause game.

Are there any guidelines for button sizes? how to draw images?

UPDATE: Same widget with a big larger size:

[stgatilov]

Yeah, and I would still be happy to change color of "manual" (yellow) and "on" (green) radio buttons when they are selected.

Same applies to "Connected" checkbox... although it sucks being a checkbox. And inactive checkbox is especially hard to notice if it toggles...

[OrbWeaver]

I think that GUI suffers from the problem that several of the early mockups had: far too many widgets for the number of actual, useful options.

The mockup I eventually came up with required only one button and one checkbox for each feature (basically "Do now" as a button and "Do always" as a checkbox). I couldn't identify any particular subfeature which required more than two widgets. I don't claim that my mockup is perfect but it does boil the options down the minimal set and might be helpful as a guide to layout.

Having separate On and Off radio buttons is very non-standard, and it's not clear how the radio buttons would interact with the button (Do you need to set radio button to On or Manual before clicking the "Do Now" button, or does it just work? I'd vote for it just working, in which case the Manual option is redundant and the On/Off buttons can be replaced by a single checkbox or toggle button).

Colour-changing radio buttons — ugh, please no. This isn't GarrettLoader and we don't use colour-changing dialog widgets anywhere else (unless they are specifically there to indicate colour, e.g. colour buttons in the Light Inspector and Themes editor).

Button images are normally 16x16 and I create them in Inkscape, but of course you can use whatever you want. Drawing an image which is still readable at such a small size can be challenging; in general you need to stick to large, flat colours and make full use of the entire space (don't leave a 1-pixel internal margin).

[stgatilov]

24 minutes ago, OrbWeaver said:

The mockup I eventually came up with required only one button and one checkbox for each feature (basically "Do now" as a button and "Do always" as a checkbox). I couldn't identify any particular subfeature which required more than two widgets. I don't claim that my mockup is perfect but it does boil the options down the minimal set and might be helpful as a guide to layout.

The idea is mostly to replace on/off radio buttons with single checkbox.

Yes, it can be done for camera sync.
Yes, it can be done for "reload from .map".

Unfortunately, it cannot be done for "hot reload" / "update entities" / whatever it is called (soon I'll get confused myself with all these names ).
User has to explicitly enable the mode even if he only wants to update manually, because we have to install a huge listener over all the entities. This mode only tracks the changes done to entities while the listener is on. It is important that when the mode is enabled, TDM has the same state of the map. That's why we should save .map file and force TDM to reload from .map file when user enables this mode.

I can switch to your style, but I need additional widget on "Hot reload" line, which would actually enable the mode.

By the way, "restart game" + "dmap" checkbox already fits your proposal.

[stgatilov]

I guess one approach would be to enable "Update entities" mode automatically when user clicks on "Restart Game" button. At this moment we can save .map in DarkRadiant, then spawn TDM or use existing or attach, in which start this map afresh of restart it. After that, the user can use the mode without problems, and cannot disable it (unless connection is lost).

On the other hand, if the user did not click "Restart Game" button, i.e. started TDM manually, enabled automation, and started the map manually, then the whole line is blocked. He can do camera sync and reload from .map though.

[peter_spy]

Stuff like load map file preference is something you probably set once and forget about it. Wouldn't it be more convenient to have most of such options in a Preferences section instead of a widget? This way you could have just a minimal set of buttons/icons over the perspective window.

[OrbWeaver]

3 hours ago, stgatilov said:

User has to explicitly enable the mode even if he only wants to update manually, because we have to install a huge listener over all the entities. This mode only tracks the changes done to entities while the listener is on. It is important that when the mode is enabled, TDM has the same state of the map. That's why we should save .map file and force TDM to reload from .map file when user enables this mode.

I can see that causing problems from a user perspective. Users won't remember/know that they have to manually start the listener, but will just expect it to work and will be confused when entity updates don't get synchronised. So anything that can be done to make it work with minimal initial setup will be ideal.

Your suggestion of starting it automatically when you spawn the game from DR makes sense. I would go further and start the entity monitoring automatically as soon as the connection was established (regardless of how the game was launched), even if the user just clicked the camera sync button. We can reasonably assume that once the user activates some synchronisation feature, they are probably going to want to use others. This means that the user doesn't have to pay the performance cost of having the listener always active if they never use game connection, but they also don't have to manually activate it.

[stgatilov]

I switched to Restart game command.

Right now there is no explicit moment when DR connects to TDM. When you click on
something, it implicitly tries to connect, and if it succeeds, then do whatever it needs to do.

Wouldn't it be better if connection was fully explicit? So when connection is missing, the whole GUI except for Start Game button is greyed out. And when connection is up, this GUI becomes active. It would definitely simplify the code.

With such change, perhaps it would make more sense to rename this "Restart Game" button to just "Connect", and let it have two checkboxes: "Restart" (default ON) and "Dmap" (default OFF):

• "restart off" + "dmap off" allows connecting to existing game without restarting it.
• "restart on" + "dmap off" always restarts game, even if it manages to connect to TDM.
• "restart on" + "dmap on" also runs dmap before starting map.

Of course, if there is no TDM process to connect to, a new one will be started in any case. And if mod/map does not match, the new game will be started in any case too.

Alternatively, I can leave "Start Game" button with "dmap" checkbox, but add Connect button somewhere, maybe hidden from normal users... Don't want to force everyone (including myself) to restart map to connect.

Another issue I noticed is that I need to do some asynchronous programming.

At this moment, all automation commands are executed synchronously. The whole GUI is blocked until the current automation command was finished. The only exception is camera synchronization, which is some sort of special case of "continuously executed" request.

For the "Restart Game" feature, I definitely don't want to block GUI while dmap and level load are running. Of course I would block connection for anything else while these tasks are going, but the user should be free to continue working on his map. Which means I need one of:

1. Separate thread. With all the problems coming from it...
2. Asynchronous programming around some sort of event loop.

I'm not very experienced with asynchronous programming, but I have worked a bit with promises and async/await in javascript. The game connection singleton already has "think" function called on timer, which is something like its own event loop embedded into event loop of wxWidgets. I can either implement something like promises, or implement something basic like a hardcoded chain of steps with waiting for response between then.

[greebo]

Re async: I'd launch the long-running task using std::async, storing the returned std::future in the class to be able to await it if really needed. Within your async method and after the actual work is done, queue an event using GlobalUserInterface().dispatch() - this event will be picked by the regular event loop, it will be executed on the UI/main thread.

You still need to take care about the things you touch in the worker thread of course, so the regular threading rules of caution still apply, but using the pattern above you can at least safely update the UI once the task is done without any polling or custom event loops.

(There are code examples in the vcs plugin I recently pushed to the master branch.)