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

In-game setting of MultipleAntennaMultiplier is snapped to 0 or 1 #594

Closed
WazWaz opened this issue Apr 30, 2016 · 10 comments

Comments

@WazWaz
Copy link

commented Apr 30, 2016

Setting it to 0.25 in the .cfg file and reloading still shows only 0 or 1.

(in-game settings dialog is cool though, BTW!)

@gnivler

This comment has been minimized.

Copy link
Contributor

commented May 4, 2016

Also seeing this;

Setting Antenna Multiplier in config file to 0.49 (didn't go to more decimals in testing) or higher sets the value to 1 in the GUI and reflects in the config file when it's written. It also goes to 0 if you set it to a string in the config file :)

@gnivler

This comment has been minimized.

Copy link
Contributor

commented May 4, 2016

from OptionsWindow.cs this seems to be by design. It would therefore make more sense to make it a toggle button than a slider.

this.mSettings.MultipleAntennaMultiplier = GUILayout.HorizontalSlider((float)this.mSettings.MultipleAntennaMultiplier, 0, 1); if(this.mSettings.MultipleAntennaMultiplier <= 0.49) { this.mSettings.MultipleAntennaMultiplier = 0; } else { this.mSettings.MultipleAntennaMultiplier = 1; }

@tomekpiotrowski tomekpiotrowski added this to the 1.7.1 milestone May 7, 2016

@amedee

This comment has been minimized.

Copy link
Contributor

commented May 7, 2016

In AbstractRangeModel.cs, GetMultipleAntennaBonus is a positive double (there is an if that checks for > 0.0 and returns 0.0 otherwise). In code I don't see it get limited by a maximum of 1.0.

I also wonder why GetMultipleAntennaBonus takes a list of omni antennas omniList and a double maxOmni, which is (at least where I found it, in AbstractRangeModel.cs) the highest value of that same omniList. How about refactoring this and only passing omniList as a variable, and let GetMultipleAntennaBonus figure out what the highest value is? (separation of responsibilities)

EDIT: so what GetMultipleAntennaBonus actually does: take the sum of all omnis, subtract the highest value, and multiply with a settings factor. Right? omniList alone contains enough information to make that calculation. It means adding

 double maxOmni =  omnis.Any() ?  omnis.Max(ant => ant.Omni) : 0.0;

If I'm talking nonsense here, sorry, I'm not a C# developer but a QA Engineer, mostly dealing with Java.

@Zhetaan

This comment has been minimized.

Copy link

commented May 10, 2016

Short version: I'd really rather not have a toggle button--I hope that's not the milestone that was added.

Long Version: Not to preach from the ranks of the plebians and peasants, here, and certainly not to be unduly confrontational to @gnivler , but given that OptionsWindow.cs has a slider for MultipleAntennaMultiplier that snaps to 0 or 1 (but says in the description that it scales linearly between those values), and AbstractRangeModel.cs scales MultipleAntennaMultiplier linearly, but doesn't limit it at 1, I'm not certain that we can make very good guesses about what behaviour is intended by design.

I definitely want non-integer possibilities: I prefer to use .25, myself. I believe there should be benefits to having multiple antennas, especially since there are both stock and real-life craft with multiples--call it my sense of functional aesthetics that says the extras should have some utility--but I wouldn't use the multiplier at all if the only choice was for omnis to be strictly additive. That bothers my sense of realism: I'd honestly love to see diminishing returns, as well, but that looks to be far in the future.

P.S.: I am aware a real-life antenna array has higher gain than the sum of the individual antennas. However, that increased gain is in one direction at the expense of the others: we have that already, in dishes.

@WazWaz

This comment has been minimized.

Copy link
Author

commented May 10, 2016

It looks "by design" in the options window, but nowhere else in the code or documentation, so I think it's just a copy-and-paste bug or typo in the options window.

@gnivler

This comment has been minimized.

Copy link
Contributor

commented May 11, 2016

Thanks for your consideration! I like the idea of a slider more than a toggle too. My original comment was just an observation from looking at the code (I'm not a dev btw, and not sure what decision if any was made for the milestone.. it's probably still to be decided. A slider can be 0 or 1 as easily as a toggle but is more versatile).

@wurmy702

This comment has been minimized.

Copy link

commented May 20, 2016

I can confirm this issue with my installation as well....

@Netrilix

This comment has been minimized.

Copy link

commented Jun 22, 2016

If this functionality is still being developed, I'd like to add my two cents. Under the current model, your second and fifth antennas help you exactly the same amount. Rather than being strictly additive, what if the multiplier instead represented a stacking penalized bonus for each subsequent antenna?

For example, if you had three Communotron 88-88s and a Comms DTS-M1 with multiplier at 0.75:

Main antenna (Comm88-88): 40Bm
Second antenna (Comm88-88): 40Bm * (0.75^1) = 30Bm
Third antenna (Comm88-88): 40Bm * (0.75^2) = 22.5Bm
Fourth antenna (CommsDTS): 50Mm * (0.75^3) = 21.09375Mm
Total range: 92.52109375Bm

Run the same example with a stronger stacking penalty, multiplier at 0.25:

Main antenna (Comm88-88): 40Bm
Second antenna (Comm88-88): 40Bm * (0.25^1) = 10Bm
Third antenna (Comm88-88): 40Bm * (0.25^2) = 2.5Bm
Fourth antenna (CommsDTS): 50Mm * (0.25^3) = 0.78125Mm
Total range: 52.50078125Bm

At the extremes of 0.0 and 1.0, the math works out exactly as it currently works.

To me, this makes the most sense, because adding a second antenna can help boost range a respectable amount, but your thirteenth antenna is pointless at any reasonable multiplier (giving you only 2.4% return at the 0.75 multiplier and a rounding error at the 0.25 multiplier).

@Zhetaan

This comment has been minimized.

Copy link

commented Jun 22, 2016

@Netrilix To clarify, the multiplier functionality has been designed to respect addition only of omnidirectional antennas, not dishes.

Speaking strictly for myself, I've always wanted to be able to use the multiplier on dishes, within reason, to replicate the Very Large Array's functionality, but that may be best addressed as a separate feature request. Getting dishes to multiply one another is a bit more tricky because dishes require targets and their ranges are so wildly different for each type.

@Netrilix

This comment has been minimized.

Copy link

commented Jun 22, 2016

That makes sense. I'll edit my example to use omnis when I get home from work, but I think my point still stands (perhaps even moreso) if we're talking about omnis only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.