Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove "master" node from networks #264

Open
S-S-X opened this issue Apr 21, 2022 · 2 comments
Open

Remove "master" node from networks #264

S-S-X opened this issue Apr 21, 2022 · 2 comments
Labels
Cleanup Cleanup of bad code or other redundant/unnecessary stuff

Comments

@S-S-X
Copy link
Member

S-S-X commented Apr 21, 2022

Network cache building currently begins from master cable which decides network id. This id is later used to find master node location in world. This whole thing was left there to keep some behavior closer to previous when caching, network and switching station stuff were refactored in #96

This master node is currently also used to check for switching station when network is executed through technic globalstep handler.
This check was left there to follow old behavior but I don't really see reason to check for switching station node at all, after network is started it could just stay active as long as its TTL is positive and ignore switching stations completely.

for network_id, network in pairs(technic.active_networks) do
local pos = technic.network2sw_pos(network_id)
local node = technic.get_or_load_node(pos) or minetest.get_node(pos)
if node.name ~= "technic:switching_station" then
-- station vanished
technic.remove_network(network_id)

Keeping master node was more just easy way to drop switching stations from network and allow switching stations to only reset TTL instead of actually running networks.
Network ID could of course be based on node position but network ID should not be guaranteed or ever used to find network position. Instead it should be just ID to get node tables and network itself should not have any master positions, it is just unnecessary and complicates things.

This network master node however is both useless and practically makes one of switching station more important/significant than others. For example while all connected switching stations are only resetting network TTL removing one that is immediately above master cable node will reset that network while removing any other switching station will not affect caches.

-- Remove network when switching station is removed, if
-- there's another switching station network will be rebuilt.
local network_id = technic.sw_pos2network(pos)
if technic.networks[network_id] then
technic.remove_network(network_id)
end

This ^^ is probably biggest problem here: decide whether there's still other switching stations in network when switching station is removed, should network be stopped or not.
Reference counters could be used but unfortunately nodes can easily disappear from world without any callbacks to Lua side... and removal of switching station is often intentional action and it is expected that machines stop immediately when last switching station is removed.

@S-S-X
Copy link
Member Author

S-S-X commented Apr 21, 2022

Not having this legacy crap makes everything easier to manage and will be overall improvement for technical implementation making feature development a lot simpler.
Comments / concerns / ideas for solutions?

@S-S-X S-S-X added the Cleanup Cleanup of bad code or other redundant/unnecessary stuff label Apr 21, 2022
@BuckarooBanzay
Copy link
Member

Not having this legacy crap makes everything easier to manage and will be overall improvement for technical implementation making feature development a lot simpler.

I'm getting to the same conclusion in a few other mods too, but yeah: removing old and unneeded stuff is always good in terms of maintainability 👍

Comments / concerns / ideas for solutions?

Can't contribute on that one, haven't touched this code in a while :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Cleanup of bad code or other redundant/unnecessary stuff
Projects
None yet
Development

No branches or pull requests

2 participants