-
Notifications
You must be signed in to change notification settings - Fork 19
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
Antenna animation and activation of one antenna, will switch others on same craft #9
Comments
hmmm... i would think of this moar as a bug, than enhancement? :P EDIT: oops... I guess first part would be a bug, second part an enhancement :D |
Any chance to get this tagged as a "bug", instead of "enhancement"? I feel the first part, as a bug, is moar important to be dealt with... ?? vOv |
I'll see if this is still an issue for me with latest release. If so, I'll see if i can split the tickets. |
It's still an issue :) I'll start working it now, though. |
Alright... I've had a look at this and it's not going to be an easy change, from what I can see. There are a lot of assumptions in the code that the vessel only has 1 Telemachus part on it and the changes spidered out very quickly. This code base is showing its age in several places. I think I'm going to try and just keep the critical bugs squashed in the 1.X version and start working on a 2.0 version. |
SO, it looks like, in the TelemachusPartModules.cs, the Telemachus part animations are toggled from detecting the status of [TelemachusPowerDrain.activeToggle]. Which looks like any toggling of that on any part's PAW, will trigger all the mutiple parts' animations. I wonder if it would be better to reverse the method, and have the PAW buttons first toggle a parts' animation (unique animation names for each part could be used to identify which part toaffect?), THEN have the animation state change, toggle the Powerdrain module?... vOv it wouldnt solve the issue with powerdrain or which part is actually used, if multiple parts are active, but it should seperate and solve the animation flip/flopping if multiple antennas are on the craft... At, least, it seems to make sense in my mind... which doesnt really mean much :P |
Unfortunately, it's not an issue of the algorithm in the function. The fields that are being manipulated are all marked
This means every instance of that class shares that data -- it's global. Simply removing the static modifier would seem the easy fix, but since that's such a fundamental field, the change ripples across a lot of code. |
Gotcha... but at least we know why toglling one part, toggles the animation on all parts ;) |
I found an issue where the "Enable/Disable Link" button in the PAW, activates the animation of ALL telemachus antennas on the craft.
Even activating the button on the non-animated TeleBlade antenna, will animate the Fustek antenna (i'm assuming if there were any other animated Telemachus antenna parts, it would affect those, too)
It also activates/deactivates the actual link state, and power consumption for those other antennas.
While I can see this might not be much of an issue with single craft, since most users would only have a single Telemachus antenna, I can see it causing issues when docking multiple craft together, where the combined craft may now have multiple telemachus antennas.
I poked around the code (i know just enuff about C# to be dangerous :P ), and it looks like when the Enable/Disable Link button is activated, it toggles the link, but then also toggles the animation. Even when toggled by a seperate instance of an antenna. I guess there should be some method, that checks for, and only toggles the animation for an antenna, from which the enable/disable command came from.
I mention this, too, becuase I would like to eventually remodel the Yagi antenna... and possibly animate it. plus, rather than hard-coding absolutes, it would be nice to leave things flexible enuff that anyone else, in the future, could mebbe make some new parts, and if animated, would need little to no changes to the plugin, due to having restrictive hard-coding in place.... make sense?
Plus, the "Enable/Disable Link" title seems a little confusing... Maybe change it to "Enable/Disable Antenna"? vOv
The text was updated successfully, but these errors were encountered: