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

Network refactoring #241

Merged
merged 2 commits into from
Nov 30, 2021
Merged

Network refactoring #241

merged 2 commits into from
Nov 30, 2021

Conversation

S-S-X
Copy link
Member

@S-S-X S-S-X commented Nov 13, 2021

Depends on #230 Merged

Main changes

  • Network overload moved to overload.lua.
  • Network building functions I've added during Partial network rewrite #96 moved to network.lua.
  • Related to previous point, cables.lua cleaned up and contains only cable registration.
  • Excluding tests there's 234 additions and 239 deletions in 4 files.

Test updates

  • Ignore sources outside technic directory for coverage report (cnc, worldgen).
  • Adds digilines fixture for tests, basically makes tests execute code inside blocks that check if digilines mod is available.
  • Reported coverage for technic mod is bit lower, this is expected because trechnic_worldgen code lines are not counted anymore.
  • Reported CNC test coverage gets bit higher and this is expected because CNC technic tests were moved there.
  • Regression test updates excluding mod code 16 files with 149 additions and 57 deletions.

There should be no functional changes in this branch, also this should not reduce available API functions.
This does however rename following functions and removes note about being exported only for tests:

-- NOTE: Exported for tests but should probably be moved to network.lua
technic.network_node_on_placenode = place_network_node
technic.network_node_on_dignode = remove_network_node

These are now renamed: technic.place_network_node and technic.remove_network_node.

@S-S-X S-S-X added Enhancement New feature or request Blocked Has unmerged dependency or some other blockage. Check details before working on this. Cleanup Cleanup of bad code or other redundant/unnecessary stuff labels Nov 13, 2021
@S-S-X
Copy link
Member Author

S-S-X commented Nov 13, 2021

Technic CNC tests could be also moved to technic_cnc/spec and make it load technic mod for tests that require technic.
Currently technic_cnc/spec contains only tests that can be executed without technic mod.
When moving keep CNC tests that do not depend on technic and just add another set for tests depending on technic.

technic_worldgen is not really tested at all, it is just loaded as dependency for technic and should stay like that.
If tests are added they should go to technic_worldgen/spec but that's kinda offtopic here.

@S-S-X S-S-X removed the Blocked Has unmerged dependency or some other blockage. Check details before working on this. label Nov 15, 2021
@S-S-X S-S-X force-pushed the network-refactoring branch from 14c217b to 2e5d1e0 Compare November 15, 2021 09:46
@S-S-X S-S-X force-pushed the network-refactoring branch from 2e5d1e0 to 792739d Compare November 15, 2021 10:11
@S-S-X
Copy link
Member Author

S-S-X commented Nov 15, 2021

Merged changes to tests into single commit... hopefully there's no need to revert those.

@S-S-X S-S-X force-pushed the network-refactoring branch from 792739d to be941d1 Compare November 16, 2021 13:25
Copy link
Member

@BuckarooBanzay BuckarooBanzay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pulled this along with latest master in the test-server and turned on my factory: no issues so far, everything was as expected 👍

@OgelGames
Copy link
Contributor

OgelGames commented Nov 25, 2021

Did some basic testing, only thing I noticed was that furnaces flicker on and off when they don't have power. Everything else seems to work correctly 👍

furnace flickering

technic-furnace-flicker

@S-S-X S-S-X force-pushed the network-refactoring branch from d6b8c4e to d5a7da1 Compare November 29, 2021 15:17
@S-S-X
Copy link
Member Author

S-S-X commented Nov 29, 2021

Did some basic testing, only thing I noticed was that furnaces flicker on and off when they don't have power. Everything else seems to work correctly 👍

Did you have something in input slot of machine? If not then this should not happen but if you had then it should be happening and always worked like that. (was added as feature for CNC machines too, visual alert about possible problem with machine that could do work but cannot start processing)

@OgelGames
Copy link
Contributor

OgelGames commented Nov 29, 2021

Did you have something in input slot of machine?

Yes.

then it should be happening and always worked like that.

It seems a bit weird for it to be doing that, it would make more sense for it to do nothing at all.

Also, I couldn't replicate it on master.

@S-S-X
Copy link
Member Author

S-S-X commented Nov 29, 2021

It seems a bit weird for it to be doing that, it would make more sense for it to do nothing at all.

Probably depends who you ask, for me it did always make sense because it allowed direct visual feedback in a way that actually makes sense for machine that attempts to start but fails immediately.
On pandorabox when I was still building basic things it was useful thing to easily notice that something is wrong with machines, either stuck or someone died in geothermal power plant...

Also, I couldn't replicate it on master.

That's weird, I just reproduced this behavior on pandorabox main server...

I guess it has something to do with timing...

Drop coverage stats for sources outside of technic directory (technic_cnc, technic_worldgen)
Update technic API functions for tests (were exported just for tests, now part of API)

Move CNC tests, use shared fixtures for tests
@S-S-X S-S-X force-pushed the network-refactoring branch from 66bc260 to 5b89200 Compare November 30, 2021 03:02
@github-actions
Copy link

Click for detailed source code test coverage report

Test coverage report for Technic CNC 86.90% in 11/14 files:

File                             Hits Missed Coverage
-----------------------------------------------------
programs.lua                   263  0      100.00%
materials/technic_worldgen.lua 32   0      100.00%
materials/init.lua             14   0      100.00%
materials/default.lua          183  0      100.00%
materials/basic_materials.lua  17   0      100.00%
digilines.lua                  55   0      100.00%
cnc.lua                        53   0      100.00%
formspec.lua                   104  7      93.69%
api.lua                        217  43     83.46%
init.lua                       19   6      76.00%
pipeworks.lua                  25   13     65.79%
materials/moreblocks.lua       0    29     0.00%
materials/ethereal.lua         0    37     0.00%
materials/bakedclay.lua        0    13     0.00%

Test coverage report for technic chests 45.24% in 6/6 files:

File          Hits Missed Coverage
----------------------------------
chests.lua    98   18     84.48%
init.lua      34   18     65.38%
register.lua  84   78     51.85%
formspec.lua  76   93     44.97%
inventory.lua 10   100    9.09%
digilines.lua 2    61     3.17%

Test coverage report for technic 61.00% in 95/95 files:

File                                      Hits Missed Coverage
--------------------------------------------------------------
max_lag.lua                               12   0      100.00%
machines/register/init.lua                15   0      100.00%
machines/register/freezer_recipes.lua     13   0      100.00%
machines/other/init.lua                   8    0      100.00%
machines/MV/solar_array.lua               12   0      100.00%
machines/MV/init.lua                      17   0      100.00%
machines/MV/grinder.lua                   17   0      100.00%
machines/MV/generator.lua                 9    0      100.00%
machines/MV/freezer.lua                   17   0      100.00%
machines/MV/extractor.lua                 17   0      100.00%
machines/MV/electric_furnace.lua          17   0      100.00%
machines/MV/compressor.lua                17   0      100.00%
machines/MV/centrifuge.lua                17   0      100.00%
machines/MV/cables.lua                    40   0      100.00%
machines/MV/battery_box.lua               17   0      100.00%
machines/MV/alloy_furnace.lua             19   0      100.00%
machines/LV/solar_array.lua               11   0      100.00%
machines/LV/init.lua                      17   0      100.00%
machines/LV/grinder.lua                   16   0      100.00%
machines/LV/generator.lua                 9    0      100.00%
machines/LV/electric_furnace.lua          15   0      100.00%
machines/LV/compressor.lua                20   0      100.00%
machines/LV/cables.lua                    40   0      100.00%
machines/LV/battery_box.lua               15   0      100.00%
machines/LV/alloy_furnace.lua             17   0      100.00%
machines/HV/solar_array.lua               11   0      100.00%
machines/HV/init.lua                      12   0      100.00%
machines/HV/grinder.lua                   17   0      100.00%
machines/HV/generator.lua                 9    0      100.00%
machines/HV/electric_furnace.lua          17   0      100.00%
machines/HV/compressor.lua                17   0      100.00%
machines/HV/cables.lua                    39   0      100.00%
machines/HV/battery_box.lua               17   0      100.00%
legacy.lua                                33   0      100.00%
items.lua                                 128  0      100.00%
crafts.lua                                133  0      100.00%
machines/LV/led.lua                       73   1      98.65%
machines/register/compressor_recipes.lua  36   1      97.30%
machines/LV/geothermal.lua                75   3      96.15%
config.lua                                49   2      96.08%
machines/register/cables.lua              95   4      95.96%
machines/register/solar_array.lua         46   2      95.83%
machines/LV/solar_panel.lua               42   2      95.45%
machines/network.lua                      397  22     94.75%
machines/register/alloy_recipes.lua       40   3      93.02%
tools/init.lua                            13   1      92.86%
machines/LV/water_mill.lua                67   6      91.78%
machines/register/grindings.lua           42   5      89.36%
machines/LV/lamp.lua                      110  14     88.71%
init.lua                                  29   4      87.88%
machines/register/grinder_recipes.lua     106  16     86.89%
util/throttle.lua                         9    2      81.82%
machines/register/machine_base.lua        165  41     80.10%
machines/LV/extractor.lua                 18   5      78.26%
machines/register/recipes.lua             62   20     75.61%
machines/register/battery_box.lua         223  72     75.59%
tools/flashlight.lua                      65   21     75.58%
machines/switching_station.lua            76   25     75.25%
machines/register/centrifuge_recipes.lua  21   7      75.00%
radiation.lua                             262  88     74.86%
effects.lua                               5    2      71.43%
machines/overload.lua                     12   5      70.59%
machines/MV/wind_mill.lua                 46   22     67.65%
machines/supply_converter.lua             95   46     67.38%
machines/other/coal_furnace.lua           2    1      66.67%
machines/other/injector.lua               72   39     64.86%
machines/switching_station_globalstep.lua 39   22     63.93%
machines/MV/hydro_turbine.lua             43   26     62.32%
machines/MV/tool_workshop.lua             56   34     62.22%
machines/register/generator.lua           122  88     58.10%
register.lua                              21   19     52.50%
tools/cans.lua                            53   48     52.48%
machines/power_monitor.lua                40   38     51.28%
machines/init.lua                         56   54     50.91%
machines/other/coal_alloy_furnace.lua     63   63     50.00%
machines/LV/music_player.lua              46   46     50.00%
tools/mining_lasers.lua                   39   40     49.37%
machines/other/constructor.lua            67   69     49.26%
tools/vacuum.lua                          19   21     47.50%
tools/tree_tap.lua                        24   27     47.06%
machines/HV/forcefield.lua                103  155    39.92%
machines/HV/nuclear_reactor.lua           119  205    36.73%
machines/HV/quarry.lua                    125  216    36.66%
machines/register/common.lua              39   75     34.21%
tools/chainsaw.lua                        40   83     32.52%
tools/multimeter.lua                      71   149    32.27%
machines/compat/api.lua                   16   34     32.00%
tools/sonic_screwdriver.lua               18   40     31.03%
machines/other/frames.lua                 184  445    29.25%
helpers.lua                               38   100    27.54%
tools/mining_drill.lua                    76   226    25.17%
machines/other/anchor.lua                 14   74     15.91%
machines/compat/digtron.lua               2    11     15.38%
tools/prospector.lua                      16   92     14.81%
machines/register/extractor_recipes.lua   6    65     8.45%

Raw test runner output for geeks:

CNC:

●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●
31 successes / 0 failures / 0 errors / 0 pending : 0.471854 seconds

Chests:

W:	Configuration: invalid key	exclude_textures
W:	Configuration: invalid key	validate_textures
●●●●●
5 successes / 0 failures / 0 errors / 0 pending : 0.03183 seconds

Technic:

●◌◌●●●◌●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●◌●●●●●
176 successes / 0 failures / 0 errors / 4 pending : 6.187382 seconds

Pending → spec/api_spec.lua @ 133
Technic API Machine registration registers my_mod:my_battery
spec/api_spec.lua:133: Battery box registration does not include all fields

Pending → spec/api_spec.lua @ 190
Technic API Machine registration registers my_mod:machine_base
spec/api_spec.lua:190: Base machine registration does not include all fields

Pending → spec/api_spec.lua @ 285
Technic API internals technic.cables TBD, misleading name and should be updated
spec/api_spec.lua:285: TBD technic.cables naming and need, see technic networks data for possible options

Pending → spec/supply_converter_spec.lua @ 78
Supply converter building overloads network
spec/supply_converter_spec.lua:78: overload does not work with supply converter

@S-S-X
Copy link
Member Author

S-S-X commented Nov 30, 2021

Flickering behavior was confirmed to be same on pandorabox and no other problems found so far I guess this is good enough so merging away. Been also waiting to be able to base latest work on master, makes everything simpler...

@S-S-X S-S-X merged commit 39d7869 into master Nov 30, 2021
@BuckarooBanzay BuckarooBanzay deleted the network-refactoring branch May 4, 2022 13:02
@Athozus Athozus added this to the 2.0.0 milestone Apr 14, 2024
@S-S-X S-S-X mentioned this pull request Dec 24, 2024
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 Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants