Telemachus-1 icon indicating copy to clipboard operation
Telemachus-1 copied to clipboard

Antenna animation and activation of one antenna, will switch others on same craft

Open StoneBlue opened this issue 6 years ago • 8 comments

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

StoneBlue avatar Mar 04 '19 02:03 StoneBlue

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

StoneBlue avatar Apr 13 '19 21:04 StoneBlue

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

StoneBlue avatar Dec 10 '19 19:12 StoneBlue

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.

StoneBlue avatar Apr 02 '20 14:04 StoneBlue

It's still an issue :) I'll start working it now, though.

sidrus avatar Apr 04 '20 13:04 sidrus

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.

sidrus avatar Apr 04 '20 14:04 sidrus

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 And yeah, this is a minor issue for now. Will be moar relevant if I end up making moar antenna parts, tho.

StoneBlue avatar Apr 08 '20 00:04 StoneBlue

Unfortunately, it's not an issue of the algorithm in the function. The fields that are being manipulated are all marked static

static public bool isActive = true;

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.

sidrus avatar Apr 08 '20 01:04 sidrus

Gotcha... but at least we know why toglling one part, toggles the animation on all parts ;) Again, if it would help, the animation names in the models themselves can all be easily changed to be unique, and/or each part assigned a unique part ID in the cfg, to use to toggle specific part animation. Just somthing to maybe keep in mind, going forward. vOv

StoneBlue avatar Apr 08 '20 03:04 StoneBlue