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

Partial network rewrite #96

Merged
merged 21 commits into from
Oct 31, 2020
Merged

Partial network rewrite #96

merged 21 commits into from
Oct 31, 2020

Conversation

S-S-X
Copy link
Member

@S-S-X S-S-X commented Sep 26, 2020

TESTING REQUIRED

ANY OLD BUGS THAT ALSO APPEAR BEFORE THIS PR SHOULD NOT BE FIXED IN THIS BRANCH

Found new bugs so far:

  • Supply converters are not working correctly, this seems to be because of machine timeout ABM change.
    actually they do seem to work but keeps blinking infotext between "no network" and actual supply info.
    Tests added ✔️
  • Placing cable between other same tier cables and machine wont add machine to network (but other cables without network and machines after those will be added) Digging and placing machine again fixes connection.
    Tests added ✔️
  • Digging/replacing connecting intermediate cable and then digging cable below machine wont remove machine from network, machine works and is part of network also drawing power normally from network.
    Tests added ✔️
  • Digging machine will remove neighbor machines from network (but wont touch cables).
    Digging machines should not affect other machines at all, machine here behaves like cable and it should not.
    Tests added ✔️
  • Placing machine adds neighbor unconnected cables to same network machine is added to.
    While placing machine should not affect unconnected cables it should still check for cables connected to another network and cause overload immediately, better sooner than later.
    This invalid network part is lost when cache is built again, only affects placement. Can be annoying for sure if you don't know how it behaves.
    Tests added ✔️
  • Digging single cable will remove machine from network even if machine has multiple connected cables to same network
    Tests added ✔️
  • Placing new cables around machine will always add machines to network, this can cause infinite duplicates in network.
    Tests added ✔️
  • Machines wont time out ever when removed from network by digging cables, this causes infotext "has no network" to be immediately replaced with "idle" or "unpowered".
    No tests added ❌
  • Networks wont overload immediately after placing machine between 2 different networks, not actually big problem but nice to have also for future updates (less aggressive overloads). This kind of worked before as caches were built a lot more often.
    Tests added ✔️
  • Supply converter inserts duplicates to RE_nodes when cable connecting to network is placed nearby.
    No tests added ❌
  • When supply converter is placed between clean networks it does not attach to networks correctly. This probably has something to do with cable tiers as when placed machines only look for first found compatible tier for cables.
    Tests added ✔️
  • Digging supply converter does not correctly remove it from all networks, whole multi tier machine support is broken.
    Tests added ✔️

Just for the record actual changed LoC is not as bad as it looks like. Excluding test related stuff which causes 16 files changed, 2434 insertions(+), actual code changes are:
9 files changed, 982 insertions(+), 745 deletions(-)
Might be possible to optimize that bit but not sure if it is worth it here, there's other upcoming updates planned anyway.

#96 (comment)

Partial power network rewrite. Testing required, WIP, back off if you fear bugs

Opening PR for partially rewritten power network handler. Breaks compatibility if some other mod depends on SS metadata, however for special cases where it cannot be easily fixed on other mod it is very easy to add metadata back and update it with ABM.

Performance

Performance seems to be slightly better that what it was, with worst case network (built badly, multiple switching stations etc.) performance boost during testing (averages for more than 8k measurements) was over 50%. With "normal" networks not that much but still better. Didn't find any case where performance would be worse than before.

Help wanted

Most probably there will be bugs, for that reason I added label "Help wanted" hoping that someone would have time to test things and write/post screenshots about problems found. maybe even fix problems.

Highlights / most important stuff

Most important improvements:

  • Switching station is not anymore network controller, actually it is not even registered as technic machine.
    It is still needed as it does add heart beat signal for network (resets network TTL countdown).
  • Every switching station now displays supply / demand just like power monitor.
  • Second ABM for switching stations removed.
  • Globalstep executes networks directly instead of through switching station.
  • Cache rebuilding is not triggered as often as it was before.
  • Cache building is cheaper, flattening arrays removed by writing indexed arrays from beginning.
  • More reusable code and is actually reused.
  • Digging and building network nodes should be bit better and cheaper but no hard proofs on this.
  • Added unit testing with busted along with some tests.

Removed metadata from switching station (moved values to network hash):

  • active
  • active_pos
  • supply
  • demand
  • battery_count
  • battery_charge
  • battery_charge_max

Metadata cleanup

Added LBM (hardcoded disable) to remove metadata from technic machines, contains metadata keys for this PR and previous merged PR's:
"LV_EU_timeout", "MV_EU_timeout", "HV_EU_timeout", "LV_network", "MV_network", "HV_network", "active_pos", "supply", "demand", "battery_count", "battery_charge", "battery_charge_max", "active"

@S-S-X S-S-X added Enhancement New feature or request Help wanted Extra attention is needed WIP Work in progress labels Sep 26, 2020
@S-S-X S-S-X linked an issue Sep 26, 2020 that may be closed by this pull request
@S-S-X

This comment has been minimized.

@S-S-X

This comment has been minimized.

@S-S-X
Copy link
Member Author

S-S-X commented Oct 4, 2020

Things noted:
technic.cables is hash table containing all cables and all machines. This has not changed.
technic.networks[id].all_nodes was hash table containing network cables but not machines. This is updated to contain also machines.

At least naming has been wrong, added FIXME note to unit tests about technic.cables, it seems that many pieces assumes that machines are there so machines cannot be just simply dropped from technic.cables.

@tuedel

This comment has been minimized.

technic/machines/network.lua Outdated Show resolved Hide resolved
@S-S-X

This comment has been minimized.

@S-S-X

This comment has been minimized.

@BuckarooBanzay

This comment has been minimized.

@S-S-X

This comment has been minimized.

@S-S-X

This comment has been minimized.

@S-S-X

This comment has been minimized.

@S-S-X

This comment has been minimized.

@BuckarooBanzay

This comment has been minimized.

@S-S-X

This comment has been minimized.

@S-S-X

This comment has been minimized.

@BuckarooBanzay

This comment has been minimized.

@S-S-X

This comment has been minimized.

@OgelGames

This comment has been minimized.

@S-S-X

This comment has been minimized.

@S-S-X

This comment has been minimized.

SX added 19 commits October 31, 2020 04:36
Update network utils, globalstep use networks, update switch ABM

Removing switching stations from network
Add fake get_modpath for busted
…y_charge_max

Remove switching station from networks

Disable few debug prints, add removed machines to LBM

clear_networks refatoring #95

Functions remove_network_node and add_network_branch #95

Fix undef network_id, remove flatten #95
Cleanup network.lua

Add minetest vector.lua

Add tests for network constructor
… already

Comment out debug prints

Network node place/dig, simple remove_network test

Luacheck warnings

Fix network check

Fix overridden global table
(#95) Place/remove network nodes. Cleanup sw_pos

(#96) SC infotext / reduce timeout ABM workload
Fix few cable place/dig issues

Tests for machine build/dig

Fix machine build/dig bugs

Fix test for machine building

Fix machines acting like cables when placed

Inline fixtures for building/digging tests

fix ignored luacheck warnings

(#105)

fix long lines

(#105)

Add more tests for bugs found

Fix tests, add minetest.get_us_time (implement using socket.gettime)
Remove rest of SP_nodes from code, it does not contain anything

Reset switch infotext, export machine_tiers

Add tests for cable building between active networks

Remove all networks when cable is placed between networks

Fix SC connectivity issues
…lace

Fix on_construct/on_destruct registration
Check network table before attempting to use it
@S-S-X
Copy link
Member Author

S-S-X commented Oct 31, 2020

Cleaned up and merging to master, users test rest best in production right

@S-S-X S-S-X merged commit f0924df into master Oct 31, 2020
@OgelGames OgelGames deleted the network-ng branch November 12, 2020 07:40
S-S-X added a commit that referenced this pull request Oct 20, 2021
(#95) Place/remove network nodes. Cleanup sw_pos

(#96) SC infotext / reduce timeout ABM workload
S-S-X added a commit that referenced this pull request Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clear_networks function should be removed
4 participants