Variables should be visible in all parts, not just variable stores.
josephcsible opened this issue ยท 7 comments
Currently, only variable stores are checked for variable cards when searching the network for them. Other parts, such as writers, display panels, proxies, and materializers, should also be searched, to avoid occasionally needing to copy a variable card for no other reason. Originally reported in the comments of #295.
I think I looked into this at one point, but the implementation was non that trivial. Perhaps something for the future.
For reference: 7bb04f5 changes some things related to variable references in networks, which may make it possible for non-variable stores to act as variable store.
I've been working on a fix for this on my fork and I have a version which works up to a point: tdaffin@590105d
The problem is that if you have an operator variable that depends on a couple of other operator variables that are themselves in parts then it does not get updated. I figure I need to something more clever with the sendVariablesUpdateEvent flag to fix that.
I figured I'd post here in case it helped, or you guys have any suggestions on how I should proceed, or if perhaps I'm just totally missing the point ;)
I've pushed a few commits to my development branch that fix the problems with operators in parts not being seen by operators that depend on them.
A comparison of my entire branch with master is here: master-1.12...tdaffin:part-variables
A new concern at this point is whether the call to onVariableContentsUpdated in PartTypeWriteBase will cause performance issues?
@tdaffin As far as I can see at the moment, you're approach seems like a good solution. Making part states also have the variable container cap seems like the logical thing to do.
The only concern I have atm are the additional calls to ' onVariableContentsUpdated' in the parts (as you have mentioned as well), which can be expensive. This should not be called when a variable updates its value, instead, this should be called when a different variable/aspect becomes activated. Could you have a look to see if these calls can be avoided? Because at the moment, this causes all variables to become updated in the network every tick, which will be quite inefficient.
(Please correct me if I misunderstood anything, as I browsed through the code rather quickly, I'll do a more thorough review when you submit your PR)
Thanks for the feedback. I figured that update was being called very often so there would be a problem.
I am having some trouble tracking down where variable/aspect activation occurs, so it might be a while before I'm ready to fix the problems and post the PR. I'm pretty new to MC modding in general so this is kind of a learning experience for me all round -- at the moment I'm struggling with the gradle setup and so forth as well.
Anyhow, it is good to know that the general approach looks good.
This is now possible thanks to #547.