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

Machines conduct digilines above #194

Merged
merged 1 commit into from
Jul 8, 2021
Merged

Machines conduct digilines above #194

merged 1 commit into from
Jul 8, 2021

Conversation

OgelGames
Copy link
Contributor

No description provided.

@OgelGames OgelGames added the Enhancement New feature or request label Jul 8, 2021
@github-actions
Copy link

github-actions bot commented Jul 8, 2021

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

@OgelGames OgelGames merged commit 8a9466a into master Jul 8, 2021
Copy link

@eshattow eshattow left a comment

Choose a reason for hiding this comment

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

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

@BuckarooBanzay
Copy link
Member

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 🤦

@S-S-X
Copy link
Member

S-S-X commented Jul 8, 2021

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 added a commit that referenced this pull request Jul 8, 2021
@S-S-X
Copy link
Member

S-S-X commented Jul 8, 2021

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

S-S-X added a commit that referenced this pull request Jul 8, 2021
@eshattow
Copy link

eshattow commented Jul 9, 2021

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
Copy link
Contributor Author

🤦

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

@S-S-X
Copy link
Member

S-S-X commented Jul 9, 2021

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
Copy link
Contributor Author

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
Copy link
Member

S-S-X commented Jul 11, 2021

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.

S-S-X pushed a commit that referenced this pull request Oct 20, 2021
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants