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

Fix torch rules #581

Merged
merged 1 commit into from Jan 30, 2022
Merged

Conversation

Desour
Copy link
Contributor

@Desour Desour commented Jan 18, 2022

Fixes #580.

The extra rule is probably there to allow using conductors on top of the node that the torch is placed to:
screenshot_20220119_002941

But this rule should not be used for vertical torches.

Test by placing torches.

@sfan5 sfan5 added the bugfix label Jan 22, 2022
@VorTechnix
Copy link

Just tested this change and it made the problem worse. Now torches aren't turned off by torches 1 block below them no matter which side they're on. This change would move the mesecon torch further away from the redstone torch's behavior in Minecraft which is the opposite of what I was trying to achieve by opening issue #580.

@Desour
Copy link
Contributor Author

Desour commented Jan 23, 2022

@VorTechnix Just to make sure, 2 nodes below the torch does work for you, right?

I don't know how exactly the redstone torches in minecraft work, but mesecons doesn't exactly replicate redstone anyways.
The problem with changing rules is always that existing builds can be broken, so this PR's intention was never to add 1-node-below rules but to remove a very weird asymmetric behaviour.
The commit that added the "behind above" rules (ac0fb91) called it "behind above", and this name makes not much sense for vertical torches. So removing it for vertical torches can be considered as bugfix and will likely not break any build.

@VorTechnix
Copy link

VorTechnix commented Jan 23, 2022

@Desour No you misunderstood the problem. It was incorrect interactions between 2 vertical torches. The behind above rule is for horizontal torches only (or if not it should be).

Here is a picture showing the expected interaction between vertical torches offset 1 block horizontally and one block vertically:
image

I achieved the result using the following code:

local torch_input_rules_unrotated_vertical   = {
	-- Somehow X and Y have been switched so global Y -1 is X -1 to mesecon torches
	vector.new(-1, 0, 1),
	vector.new(-1, 0, -1),
	vector.new(-1, 1, 0),
	vector.new(-1, -1, 0)
}

@VorTechnix
Copy link

However, upon further testing, the above code breaks the interaction between vertical torches and horizontal ones. This is most frustrating.

@Desour
Copy link
Contributor Author

Desour commented Jan 23, 2022

@Desour No you misunderstood the problem. It was incorrect interactions between 2 vertical torches.

Torches don't interact with other torches differently than with any other mesecons things. Could you please elaborate?

The behind above rule is for horizontal torches only (or if not it should be).

I agree, and this PR does this.

Here is a picture showing the expected interaction between vertical torches offset 1 block horizontally and one block vertically: [img]

I disagree about that being expected behaviour.
As already said:

this PR's intention was never to add 1-node-below rules

Because of the bugginess of the current 1-node-below rules (and because it only works in 1 of 4 directions), it is very unlikely that builds make use of it. It is more likely that there are builds where your suggested rules cause unintentional torch deactivation.

@VorTechnix
Copy link

Because of the bugginess of the current 1-node-below rules (and because it only works in 1 of 4 directions), it is very unlikely that builds make use of it. It is more likely that there are builds where your suggested rules cause unintentional torch deactivation.

I agree to an extent but then what about builds like this (torch based AND gate) that require side and below activation of vertical torches?
image

@VorTechnix
Copy link

Anyway for now the changes you've made in this PR remove inconsistency which I guess technically solves the issue I opened. That said I think there should be some discussion about some sort of implementation of node below logic (maybe as a different torch).

@sfan5 sfan5 merged commit 4eea083 into minetest-mods:master Jan 30, 2022
@Desour Desour deleted the fix_torch_input_rules_vert branch January 30, 2022 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Torch logic directional bug
3 participants