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

Feat: effector with conductor #648

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

snowyu
Copy link

@snowyu snowyu commented Jul 10, 2023

Issue: #638

Due to the immediate changes made by the conductor, while the effector is lagged and executed in a queue, the conductor must always swap_node_force to avoid an infinite loop.

The best solution is to refactor the effector and conductor code to allow for power transmission to also be executed in a queue. In my opinion, the conductor is a special type of effector and should not be treated differently. The conductor should be derived from the abstract effector class, and its default action should be to check and swap_node.

@Desour
Copy link
Contributor

Desour commented Jun 18, 2024

The best solution is to refactor the effector and conductor code to allow for power transmission to also be executed in a queue. In my opinion, the conductor is a special type of effector and should not be treated differently. The conductor should be derived from the abstract effector class, and its default action should be to check and swap_node.

There are builds that require to have instantaneous conductors. And idk how you could avoid infinite loops.
Imo, the mechanics of conductors should not be changed in such a way.


Code has some style issues. I.e. mesecons doesn't usually use camelCase.


Doing things like checking if it's a conductor, and then doing things with the node as effector, doesn't make much sense to me.


It is important that the effector part of a node is only triggered if the signal comes via an effector rule, and that the conductor part is only used if the signal comes via a conductor rule. It doesn't look like the PR does this.


If someone wants to have a conductor, that also does something else when it changes state, they can copy the conductor's rules to their effector rules.


I should've given feedback earlier, sry.
If you don't want to work on this anymore, I might pick this up sooner or later (or much later).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants