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 a few bugs that caused effectors not to turn off sometimes (rules_li... #134

Merged
merged 1 commit into from Mar 20, 2014

Conversation

Ekdohibs
Copy link
Member

@Ekdohibs Ekdohibs commented Jan 5, 2014

...nk is evil!)

@Jeija
Copy link
Collaborator

Jeija commented Jan 5, 2014

Could you please give an example where this was an issue?

@Ekdohibs
Copy link
Member Author

Ekdohibs commented Jan 5, 2014

The issue is with nodes that have a "special" field (that allows or disallows connection depending on its value), and that have multi-rules, for example, the new wires. What happens is that sometimes effectors don't get disabled, because it thinks the conductor is on where it wasn't. Moreover, this pull improves speed, because now, you use the rule you had, and else, you would search in the inputrules which rule it was.

Now, for the issue:
screenshot_1640428256

When you turn any of them off, the connected lightstone stays on. However, if the other wire is off at that time, the connected lightstone turns off correctly.

@Jeija
Copy link
Collaborator

Jeija commented Jan 5, 2014

Unfortunately, this pull request brakes some basic functionality (just tested it):
screenshot_1640879759

  • Place two switches
  • Turn both on
  • Turn off the left one
    --> Wire is off, even though it should be on

For some reasons, doesn't work in all cardinal directions, just try several circuits, one of them will work.
That part of mesecons is very important and easily breakable code, that's why I am so careful with it...

This is propably a good fix, but it requires a lot of testing to get merged, so prepare to wait some time to get this in.

@Ekdohibs
Copy link
Member Author

Ekdohibs commented Jan 5, 2014

It should now be fixed again, I wasn't able to reproduce the bug anymore. The main change compared to the current code is that the function returns mesecon:invertRule(outputrule) and not inputrule. Indeed, what is needed is the rule for the conductor that powers this place, not the rule for the conductor that is powered. It is inverted since we need the rule as if it were for the input (there IS a difference between inputrule and mesecon:invertRule(outputrule) if outputrule has a "special" field)

@VanessaE
Copy link

VanessaE commented Jan 5, 2014

For the sake of testing, this pull is on my creative, survival, and "mg" servers. As of an hour or so ago, mesecons was fully up-to-date, besides. Or at least I'm pretty sure it was. :-)

digitalaudioconcepts.com ports 30000, 30001 and 30003, respectively.

Also on a related note, the new wires are on creative (30000), so that should give us the desired testing with those, as well.

@Uberi
Copy link
Collaborator

Uberi commented Jan 6, 2014

Some simple logical circuits worked, basic traps/farms/harvestors fully functional, advanced stuff like minicomputer broken, but probably due to some other change, I haven't played with them for a while now. Looks pretty good to me.

@Ekdohibs
Copy link
Member Author

Perhaps now it is told it works, it could be merged? (Uberi, tell me if https://github.com/Novatux/minetest-mod-mesecons/tree/test fixes it, the bug may have been there)

@Uberi
Copy link
Collaborator

Uberi commented Jan 10, 2014

The advanced machines still don't work, but I suspect we broke or changed something with Luacontrollers a while back, so this doesn't seem to be cause of the issue. +1 for merging!

Jeija added a commit that referenced this pull request Mar 20, 2014
Fix a few bugs that caused effectors not to turn off sometimes
@Jeija Jeija merged commit 2cab6aa into minetest-mods:master Mar 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants