mt-mods / technic

Technic mod for Minetest
18 stars 25 forks source link

Machines conduct digilines above #194

Closed OgelGames closed 3 years ago

github-actions[bot] commented 3 years ago
Click for detailed source code test coverage report ### Test coverage report for Technic CNC 79.43% in 8/8 files: ``` File Hits Missed Coverage ----------------------------------------------------- api.lua 157 83 65.42% cnc.lua 50 3 94.34% digilines.lua 39 8 82.98% formspec.lua 103 8 92.79% init.lua 19 6 76.00% materials.lua 174 94 64.93% pipeworks.lua 25 13 65.79% programs.lua 263 0 100.00% ``` ### Test coverage report for technic 9.55% in 10/102 files: ``` File Hits Missed Coverage -------------------------------------------------------------- config.lua 45 4 91.84% crafts.lua 0 86 0.00% effects.lua 0 3 0.00% helpers.lua 0 116 0.00% init.lua 0 30 0.00% integration_test.lua 0 24 0.00% items.lua 0 51 0.00% legacy.lua 0 7 0.00% machines/HV/battery_box.lua 0 6 0.00% machines/HV/cables.lua 9 26 25.71% machines/HV/compressor.lua 0 6 0.00% machines/HV/electric_furnace.lua 0 6 0.00% machines/HV/forcefield.lua 0 213 0.00% machines/HV/generator.lua 9 0 100.00% machines/HV/grinder.lua 0 6 0.00% machines/HV/init.lua 0 12 0.00% machines/HV/nuclear_reactor.lua 0 266 0.00% machines/HV/quarry.lua 0 304 0.00% machines/HV/solar_array.lua 0 6 0.00% machines/LV/alloy_furnace.lua 0 6 0.00% machines/LV/battery_box.lua 0 6 0.00% machines/LV/cables.lua 10 26 27.78% machines/LV/compressor.lua 0 10 0.00% machines/LV/electric_furnace.lua 0 6 0.00% machines/LV/extractor.lua 0 13 0.00% machines/LV/generator.lua 0 7 0.00% machines/LV/geothermal.lua 0 56 0.00% machines/LV/grinder.lua 0 7 0.00% machines/LV/init.lua 0 16 0.00% machines/LV/lamp.lua 0 107 0.00% machines/LV/music_player.lua 0 81 0.00% machines/LV/solar_array.lua 0 6 0.00% machines/LV/solar_panel.lua 0 27 0.00% machines/LV/water_mill.lua 0 47 0.00% machines/MV/alloy_furnace.lua 0 6 0.00% machines/MV/battery_box.lua 0 6 0.00% machines/MV/cables.lua 10 26 27.78% machines/MV/centrifuge.lua 0 6 0.00% machines/MV/compressor.lua 0 6 0.00% machines/MV/electric_furnace.lua 0 6 0.00% machines/MV/extractor.lua 0 6 0.00% machines/MV/freezer.lua 0 6 0.00% machines/MV/generator.lua 0 7 0.00% machines/MV/grinder.lua 0 6 0.00% machines/MV/hydro_turbine.lua 0 44 0.00% machines/MV/init.lua 0 17 0.00% machines/MV/lighting.lua 0 170 0.00% machines/MV/power_radiator.lua 0 96 0.00% machines/MV/solar_array.lua 0 7 0.00% machines/MV/tool_workshop.lua 0 73 0.00% machines/MV/wind_mill.lua 0 45 0.00% machines/compat/digtron.lua 0 13 0.00% machines/init.lua 0 86 0.00% machines/network.lua 193 163 54.21% machines/other/anchor.lua 0 79 0.00% machines/other/coal_alloy_furnace.lua 0 94 0.00% machines/other/coal_furnace.lua 0 3 0.00% machines/other/constructor.lua 0 103 0.00% machines/other/frames.lua 0 551 0.00% machines/other/init.lua 0 8 0.00% machines/other/injector.lua 0 85 0.00% machines/power_monitor.lua 0 57 0.00% machines/register/alloy_furnace.lua 0 30 0.00% machines/register/alloy_recipes.lua 0 27 0.00% machines/register/battery_box.lua 0 238 0.00% machines/register/cables.lua 168 49 77.42% machines/register/centrifuge.lua 0 6 0.00% machines/register/centrifuge_recipes.lua 0 25 0.00% machines/register/common.lua 0 114 0.00% machines/register/compressor.lua 0 6 0.00% machines/register/compressor_recipes.lua 0 33 0.00% machines/register/electric_furnace.lua 0 6 0.00% machines/register/extractor.lua 0 6 0.00% machines/register/extractor_recipes.lua 0 71 0.00% machines/register/freezer.lua 0 6 0.00% machines/register/freezer_recipes.lua 0 12 0.00% machines/register/generator.lua 91 114 44.39% machines/register/grinder.lua 0 6 0.00% machines/register/grinder_recipes.lua 0 100 0.00% machines/register/grindings.lua 0 39 0.00% machines/register/init.lua 0 22 0.00% machines/register/machine_base.lua 0 166 0.00% machines/register/recipes.lua 0 78 0.00% machines/register/solar_array.lua 0 30 0.00% machines/supply_converter.lua 75 66 53.19% machines/switching_station.lua 0 79 0.00% machines/switching_station_globalstep.lua 0 58 0.00% max_lag.lua 0 10 0.00% radiation.lua 0 138 0.00% register.lua 20 20 50.00% tools/cans.lua 0 71 0.00% tools/chainsaw.lua 0 115 0.00% tools/flashlight.lua 0 68 0.00% tools/init.lua 0 14 0.00% tools/mining_drill.lua 0 268 0.00% tools/mining_lasers.lua 0 65 0.00% tools/multimeter.lua 0 208 0.00% tools/prospector.lua 0 101 0.00% tools/sonic_screwdriver.lua 0 51 0.00% tools/tree_tap.lua 0 38 0.00% tools/vacuum.lua 0 32 0.00% util/throttle.lua 0 11 0.00% ``` ### Raw test runner output for geeks: CNC: ``` ●●●●●●W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results ●W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results ●●●●●●●W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results ●W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results ●W: InvRef:get_list returning list src as reference, this can lead to unxpected results ●●●●W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list src as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results W: InvRef:get_list returning list dst as reference, this can lead to unxpected results ●● 22 successes / 0 failures / 0 errors / 0 pending : 0.198217 seconds ``` Technic: ``` E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: E: INVALID MINETEST CONFIGURATION FILE PATH FOUND: E: spec/fixtures/minetest.cfg E: E: PLEASE CHANGE NAME OF FILE TO BE minetest.conf: E: spec/fixtures/minetest.conf E: E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! ●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: E: INVALID MINETEST CONFIGURATION FILE PATH FOUND: E: spec/fixtures/minetest.cfg E: E: PLEASE CHANGE NAME OF FILE TO BE minetest.conf: E: spec/fixtures/minetest.conf E: E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! ●●●●●●●●●●●●●●●●●E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: E: INVALID MINETEST CONFIGURATION FILE PATH FOUND: E: spec/fixtures/minetest.cfg E: E: PLEASE CHANGE NAME OF FILE TO BE minetest.conf: E: spec/fixtures/minetest.conf E: E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! E: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! ●●●●●◌●●●●● 57 successes / 0 failures / 0 errors / 1 pending : 0.160291 seconds Pending → spec/supply_converter_spec.lua @ 99 Supply converter building overloads network spec/supply_converter_spec.lua:99: overload does not work with supply converter ```
BuckarooBanzay commented 3 years ago

I don't understand how it works but at a quick glance but this is a repeat of the existing rule?

Uhm, yeah: it is, not sure how 4 eyes could have missed that :facepalm:

S-S-X commented 3 years ago

Well, simply just revert but where did idea came from as that was exact rule added long ago? Is there possibly problems with it currently?

S-S-X commented 3 years ago

Actually it seems that is to conduct from above, that was left out last intentionally. Don't remember what were reasons....

I mean change (with comments) were, "along y above":

        {x =  0, y = -1, z = 0}, -- along y below
+       {x =  0, y = -1, z = 0}, -- along y above

edit. here #43 and I think it was left out because cables should not connect to most machines from above and custom rules were primarily for digicables while also keeping it as backwards compatible with in world builds as possible, fix for that #124

eshattow commented 3 years ago

It seems odd visually for Supply Converter then when power digi cable goes to the input (top) but digiline signal on that power digi cable is not connected to the Supply Converter. edit: Also, since HV Digi Cable and MV Digi Cable one on top of the other share digiline, why would machines not? I'm rather wondering why digiline would not be connected above than why we should keep the same behavior. I've made several builds where I have to work around this inconsistent behavior (it works here for power digi cables, but machines do not get the signal there?)

At least for Supply Converter it should connect from above so the behavior matches what the player is looking at, a power digi cable goes into the top input it should then have a connection to digiline and power not just power only.

OgelGames commented 3 years ago

🤦

I still think it makes sense for machines to conduct above, because they can be powered from above.

S-S-X commented 3 years ago

It seems odd visually for Supply Converter then when power digi cable goes to the input (top) but digiline signal on that power digi cable is not connected to the Supply Converter.

Yes, there's few machines that should for sure allow connection from above. At least ones that also allow connecting cables from above, but if going with cable connections then for most machines / generic rules machines should not connect digilines from above.

🤦

I still think it makes sense for machines to conduct above, because they can be powered from above.

These are bugs and not meant to work this way. They can be powered from above because of buggy connections, just did not have time to fix that. There's also no connection overlay textures on top of machines other than pipeworks for some machines. You can also power machines from front panel but this is also bug and machines should not allow this.

That reasoning is not valid in my opinion as you're basically saying that connection should be allowed because cable connections are also broken.

That said when I added custom digiline rules only reason why I did not want to add connection directly above was to avoid problems with excessive messages sent to digiline network where you do not expect those but other than that I'm not against adding connection from above. Just think how many problems it might cause with current networks in world, how many will contraptions would cause higher digiline usage (and added penalty if connection would be added)? Would it be low enough for it to be acceptable?

These connections are also documented, also if you missed it linking again to bug fix ticket #124

For most machines intended cable connections are similar to "base machine" (or even less than, like bb): local connect_default = {"bottom", "back", "left", "right"}, few special cases allows cable connection from above.

OgelGames commented 3 years ago

Just think how many problems it might cause with current networks in world, how many will contraptions would cause higher digiline usage (and added penalty if connection would be added)? Would it be low enough for it to be acceptable?

I think it would be fine, after all this is only to connect machines to digilines, machines don't conduct digilines through themselves.

You can also power machines from front panel but this is also bug and machines should not allow this.

And yet they conduct digilines on the front...

Also, we're forgetting that digilines also exists separate from cables, in particular digimese that conducts downwards.

S-S-X commented 3 years ago

And yet they conduct digilines on the front...

Because that is one of default digiline connections but directly above and below are not, I've even written comment about that in technic digiline definition.

Also, we're forgetting that digilines also exists separate from cables, in particular digimese that conducts downwards.

We are not forgetting that, it just is most common to have cables below machines and very uncommon to have cables above machines. This is partly due to how cables are supposed to work.

Basically for new connections below the machine was clearly safest option that made newly added digicables a lot more useful for connecting machines normal way.