saviola777 / haxball-headless-manager

Suite of management scripts for headless haxball hosts, including plugins with dependency management
MIT License
7 stars 3 forks source link

Plugin dependencies don't get loaded when hot loading #24

Closed morko closed 4 years ago

morko commented 4 years ago

I was implementing support for the hot loading of plugins for haxroomie and when I tried to hot load sav/roles it produced the following log:

priv> reload
LOADING CONFIG from /home/oskari/.haxroomie/config.js
RELOAD CONFIG Rooms (priv) were modified.
RELOAD CONFIG Setting plugin config for priv.
priv> 2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Plugin sav/roles loaded from saviola777/hhm-plugins@development 
2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Plugin sav/commands loaded from saviola777/hhm-plugins@development 
2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Plugin sav/help loaded from saviola777/hhm-plugins@development 
2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Plugin sav/players loaded from saviola777/hhm-plugins@development 
2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Loading plugin sav/roles and its dependencies 
2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Loading the following plugins: 
2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  sav/commands, sav/help, sav/players, sav/roles 
2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Plugin loaded successfully: sav/commands 
PLUGIN LOADED sav/commands
priv> 2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Plugin loaded successfully: sav/help 
PLUGIN LOADED sav/help
priv> 2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Plugin loaded successfully: sav/players 
PLUGIN LOADED sav/players
priv> 2019-10-27 10:09:20 [debug]: [priv] [INFO] [INFO HHM]:  Plugin loaded successfully: sav/roles 
PLUGIN LOADED sav/roles
priv> plugins
hr/always-one-admin (enabled)
hr/pause (enabled)
hr/kickban (enabled)
sav/roles (enabled)

It seems like the dependencies do not get loaded correctly even though it claims so.

morko commented 4 years ago

Ups I realized this issue should be at the haxball-headless-manager repo. Sorry.

saviola777 commented 4 years ago

np, I'll look into this as soon as I can.

saviola777 commented 4 years ago

How to reproduce this? It seems to restart the room when I call reload.

room1> reload                                                                                                           
LOADING CONFIG from /media/shared/source/git/haxball-headless-manager/local/haxroomie/config-testing.js                 
RELOAD CONFIG Rooms (room1) were modified.                                                                              
CLOSING ROOM room1      
ROOM CLOSED room1           
STARTING ROOM room1
[…]

Does !plugin reload sav/roles of the sav/plugin-control plugin produce the same output?

I'll push my local changes later today, maybe it's something that has been fixed in the meantime.

morko commented 4 years ago

Ah yes sorry ive made the changes only at the feature branch https://github.com/morko/haxroomie/tree/feature/hot-reload-plugins. To reproduce start a room with a config that doesnt have the roles plugin and then edit it to include the roles plugin and do reload.

saviola777 commented 4 years ago

I think I found the problem, it becomes clear if you subscribe to the pluginRemoved HHM event – the dependencies get loaded properly, but then this code removes all plugins not explicitly listed in the plugin config.

I'm not sure why I didn't add dependency checks in my removePlugin() function though, I think only disabled plugins should be removable. A quick fix for this problem in your code would be calling disablePlugin(pluginName, false) for each of the plugins not in the pluginConfig and then checking if the returned array is empty. Only if it is not empty (i.e., if the plugin could be disabled, which is only the case if no other plugins depend on it), you would then call removePlugin().

I will adjust my code so that removePlugin will try to do this internally and return false if other plugins depend on it.

morko commented 4 years ago

Oh ok thanks for debugging my mistakes then. :D I will look into it when i have time.

saviola777 commented 4 years ago

It now "works" in that it does not allow removal of dependencies, this leads to strange results sometimes though because depending on the order in which plugins are removed, it works or it does not work as expected.

Not sure what the cleanest way here is, but the simplest (and dumbest) way is probably iterating over the plugins that are not in the pluginConfig and trying to remove them until nothing was removed for one iteration. :D

Or maybe going over the plugins not in the pluginConfig and checking if they are a direct or indirect dependency of one of the plugins listed in the pluginConfig (getDependentPlugins(pluginIdOrName, true)), and removing them if not.

saviola777 commented 4 years ago

I will close this for now, maybe we can discuss how to solve this cleaner sometime, or maybe it will work if you change how you remove plugins during hot loading.