Request for feedback - auto-reload feature

[jonri]

I've recently started working on making some small quality-of-life improvements to DarkRadiant, and I think this one will be of interest of anyone who tends to make a lot of custom models or materials.  

Instead of manually going to "Reload Models", "Reload Materials", or "Reload Skins", DarkRadiant will automatically monitor your models, materials, and skins folders inside your FM for changes.  Whenever a file gets added, removed, or modified, the appropriate reload command will happen automatically.

To try this out, I've pre-built a Windows version available here.  If you're on Linux or would prefer to build it yourself, grab the feature branch here.

All feedback is welcome, but I'm specifically looking for the following:

This is my first time building DR for Windows.

• Let me know if you try the pre-built version and it doesn't work for you!
• I seriously doubt this will eat your map, but definitely let me know if it does.  Always keep good backups, doubly so when beta testing...

I've started with reloading of Models, Materials, and Skins because they seemed the most useful to me, and as far as I have seen they can already handle getting updated at any moment (such as when you've got the "Choose Model" dialog open).

• What other reloads would be useful?  I'm leaning towards skipping Readables and Particles since they have GUI editors.  I imagine mappers don't hand-edit those as often, and the GUI editors would likely have to be changed in order to deal with external changes getting reloaded while they were open.
• That leaves "Reload Defs" currently unimplemented.  I could see this being valuable to have, if it's helpful to others I'd be more willing to do the work for the cases where things like the "Create Entity" dialog need to deal with unexpected reloads.

The auto-reload feature is currently always enabled.  I'm already planning to add an option in the Preferences dialog to enable or disable the feature.

• Should this feature be on or off by default?
• Should auto-reload of each type (Models, Materials, Skins, etc) be enabled/disabled individually, or one checkbox for all of them?

Currently this only watches your FM folder for changes.  Is there any value in watching the TDM install folder as well?

• If we watch too many folders at a time, it's possible to run into performance problems or maybe hit a limit on the maximum number of watchers.
• If this would be useful for development of core assets, I would look into whether the above causes any problems.  Otherwise I'll stick with just watching the FM folder.

Windows does sneaky things behind your back.

• I primarily run Linux, and watching files for changes works just the way you'd expect it to.
• When testing this on Windows, sometimes files would get seen as modified the first time you accessed them, and then not again afterwards.  For example, loading a map would cause one model to say it was being modified, triggering the reload models.  Afterwards, loading the map again would not do this.  I'm guessing maybe the virus scanner is meddling with things here.  
• If you experience a lot of spurious reloading that you don't expect, to the point of it being disruptive, let me know and I can try to find a workaround.
• Apparently DarkRadiant runs on Mac?  If anyone's using it there, please let me know so we can at least try to make sure this runs OK there.

Currently this only auto-updates DarkRadiant, not a running instance of TDM.

• With the new game connection, we could probably have DR trigger the reloading of these items in-game whenever they are reloaded in DR (both from auto-reload and manual reload).  I believe there was a proof-of-concept of something similar using an external script.  In this case it could happen directly through DR instead. Would this be worth pursuing?

Is there anything else I've overlooked?  While I've dabbled with mapping for a couple years now, I haven't (yet) developed an FM to completion.

[stgatilov]

I would say if DR monitors assets, it should monitor all of them: both core assets and FM assets. I'd definitely skip monitoring files inside zips, since it is a very different story. But if there are some unpacked core assets, it would be better to monitor them too (at least TDM devs would be happy). DarkRadiant should have the set of paths where assets are searched for, the most correct approach would be to monitor them all.

As for monitoring too many files. I used watchdog module in Python, it watched over 20K core assets without any problem on Windows. Perhaps it depends on which OS API is used by the library of choice. I hope you do use some third-party library for watching over files, don't you?

Speaking of preferences, I would prefer having one checkbox per type of assets. This way we can avoid arguing over what should and what should not be reloaded automatically.

As for TDM game and game connection. The best approach would be to automatically reload assets in TDM when they are reloaded by DR if game is currently connected.
Implementation-wise, DR functions for assets reloading should emit events like "I have just reloaded skins". The game connection should subscribe to these events when it gets enabled, and unsubscribe when it gets disabled. On such event, it should execute reloadSkins or reloadDecls or whatever other console command.

P.S. I must warn that my opinion is hardly decisive: I'm neither DR developer nor a mapper.

[peter_spy]

6 hours ago, jonri said:

I think this one will be of interest of anyone who tends to make a lot of custom models or materials.  

As a modeller and texture / material maker, I use DR just to insert a model into my test scene with proper lighting setup, or apply new tiling material to a test brush. Since DR has some basic rendering only, I have to rely on TDM exclusively to prototype a model or material properly. So that's where these options would be really useful in this context. I understand that this system makes sense with the ongoing automation effort, so it will be available for TDM too, at some point.

I only wonder whether just "cutting the middleman" and implementing this in TDM in the first place would be simpler (for both devs and users).

Edited January 9, 2021 by peter_spy

[OrbWeaver]

8 hours ago, stgatilov said:

I would say if DR monitors assets, it should monitor all of them: both core assets and FM assets. I'd definitely skip monitoring files inside zips, since it is a very different story. But if there are some unpacked core assets, it would be better to monitor them too (at least TDM devs would be happy). DarkRadiant should have the set of paths where assets are searched for, the most correct approach would be to monitor them all.

Agreed. Unless there is a strong reason to only monitor a particular type of asset, it would be better to have a single option "Monitor for asset changes" which is either on or off.

Quote

Speaking of preferences, I would prefer having one checkbox per type of assets. This way we can avoid arguing over what should and what should not be reloaded automatically.

I don't agree with this, it seems like the common "We can't agree on the correct behaviour so we'll just dump it onto the user through dozens of preferences" approach. Also the DarkRadiant preferences dialog is somewhat limited in terms of UI layout (it's not manually designed, but automatically generated based on adding particular options in code), and having a vertical list of 10 different checkboxes is going to become very cumbersome. Of course this problem could be solved with additional development work, but as above, I don't think this effort should be invested until there is a proven need for such fine-grained control.

Quote

As for TDM game and game connection. The best approach would be to automatically reload assets in TDM when they are reloaded by DR if game is currently connected.

7 hours ago, peter_spy said:

I only wonder whether just "cutting the middleman" and implementing this in TDM in the first place would be simpler (for both devs and users).

I strongly prefer Peter's approach on this one. Using DR as a middleman responsible for updating the game in response to filesystem changes (which are not in themselves DarkRadiant-made changes) is just complicating the process. It would be better to have TDM itself monitor for changes in its own way, which also allows the user to update assets in-game even if DarkRadiant is not running (e.g. they are just doing a quick run-through to adjust some sounds or models without bothering to load the editor).

[jonri]

14 hours ago, stgatilov said:

As for monitoring too many files. I used watchdog module in Python, it watched over 20K core assets without any problem on Windows. Perhaps it depends on which OS API is used by the library of choice. I hope you do use some third-party library for watching over files, don't you?

Good to know, thanks!  And yes, since DR uses wxWidgets, the logical choice was to use wxFileSystemWatcher.

14 hours ago, stgatilov said:

As for TDM game and game connection. The best approach would be to automatically reload assets in TDM when they are reloaded by DR if game is currently connected.
Implementation-wise, DR functions for assets reloading should emit events like "I have just reloaded skins". The game connection should subscribe to these events when it gets enabled, and unsubscribe when it gets disabled. On such event, it should execute reloadSkins or reloadDecls or whatever other console command.

That's exactly what I was thinking originally.  To the others' point, though, the ideal situation would indeed be to cut out the middleman and have TDM reload the files directly as well.  I suspect it would be a lot more work to do this directly in TDM, though, and that's why it hasn't been done.  Since the DR => TDM command passing interface is already there, the "middleman" method shouldn't be hard to do, with the hope that it might be replaced with a better method directly through TDM someday.

 

6 hours ago, OrbWeaver said:

I don't agree with this, it seems like the common "We can't agree on the correct behaviour so we'll just dump it onto the user through dozens of preferences" approach. Also the DarkRadiant preferences dialog is somewhat limited in terms of UI layout (it's not manually designed, but automatically generated based on adding particular options in code), and having a vertical list of 10 different checkboxes is going to become very cumbersome.

This is the direction I'm leaning.  One checkbox to enable/disable auto-reloading, possibly 2 if there does turn out to be a reason to toggle the core assets separately.