HDIAndrew / EFS

12 stars 0 forks source link

Unitspot.dat Algorithm is providing lower camo than it should. Should be Add instead of Multiply #182

Closed Mastnosis closed 7 months ago

Mastnosis commented 11 months ago

Description: The algorithm in UnitSpot.dat is giving lower than expected terrain modifiers. For example, based on the examples given, a unit in Trees and Grass terrain is getting lower concealment than a unit in Grass only. This seems counter intuitive.

Expected behavior: (Trees (1.8) + Grass(1.5)) / 2 = 1.65 (a value between trees and grass)

Actual behavior: 1.8 * 1.5 / 2 = 1.35 ( trees should enhance camouflage not reduce it.)

Game version: 1.51 final

Steps to reproduce: View unitspot.dat file

Additional information:

floralpond commented 9 months ago

Thanks for the input, it does seem like camo might be reduced by trees in this particular scenario, but these values get rounded up (so they both round up to 2), which means there's actually no difference in camo in the end result.

That being said, I think most would agree with you that Trees should increase camo measurably, and this is why these files are completely moddable - so people can fine tune some ways in how the game works to their liking. Vanilla EFS is not really meant to be the best possible version of the game, and we actually spend most of our time giving more power to modders rather than perfecting vanilla.

One example of a mod that has changed how camo and spotting works is Nova2.4. In Nova2.4, a grass tile has a 0.8 value, and trees have a value of 3.4 (both on Normal planets with Foot move types). So a plain old grass tile would have a camo factor of 1.0 (0.8 / 1 = 0.8, rounds up to 1), while a grass and Tree tile would have a camo factor of (0.8 * 3.4 / 2 = 1.36, rounds up to 2). This means that a 5 camo unit would have effectively a 5 camo rating on plain old grass, but would have a 10 camo rating in trees.

Mastnosis commented 9 months ago

While this particular case may not make a practical difference it must make a difference in some, potentially important cases. My example only gives two terrain modifiers but it sounds like there could be 3+ in which the effect is even stronger. And unless mods can modify the algorithm (which I'm assuming they can't) wouldn't it make more sense to correct the algorithm. From what I have gleaned from the documentation, this should be as simple as changing a single character from * to +.

And while we are on the topic, wouldn't it make more sense to round the end result rather than the camo factor. So in your example, instead of rounding 1.36 up to 2 5 making 10, make it 1.36 5 = 7 after rounding up from 6.8. This would give more nuance to camo. Should be as simple as changing one line of code though its game ramifications could be hard to calculate.

Matt-Caspermeyer commented 9 months ago

Note that there are issues with the current camouflage system in that values are not updated after the unit is created.

We have fixed this in our development build, but the multiplicative camouflage system is going to be revisited at some point in the future and I agree that it should be additive, instead of multiplicative.

Since the current development build is mostly focusing on fixing bugs and not making algorithm changes, unfortunately, you're going to have to wait until we get to that development phase before the algorithm is changed.

Mastnosis commented 8 months ago

This isn't a bug... what? When something is not acting as expected is that not the definition of a bug? ( Or are you saying this is currently as intended. What, what?) :)

I can understand you not wanting to fix something right away that can majorly impact the game behavior, expected or not. If you are going to implement a feature, make sure it is doing what you want. That may take some thought. But lets still call a spade a spade. :)

Matt-Caspermeyer commented 8 months ago

This isn't a bug... what? When something is not acting as expected is that not the definition of a bug? ( Or are you saying this is currently as intended. What, what?) :)

I can understand you not wanting to fix something right away that can majorly impact the game behavior, expected or not. If you are going to implement a feature, make sure it is doing what you want. That may take some thought. But lets still call a spade a spade. :)

I'm sure we're getting into semantics at this point. The algorithm (as originally designed by the developers) now works correctly in our development builds so it is not a bug. This is not necessarily the algorithm that you and I prefer, but at this juncture an algorithm change requires more thought and development that, unfortunately, the team has decided to push to a later date. It is not as simple as changing a "*" to a "+" as you point out, and that's why it requires thought and some actual time to develop a change that works better than the current system.

I will say, though, that if you would like to join the team and help work on stuff like this, please contact @HDIAndrew as we need all the help we can get!

floralpond commented 8 months ago

This isn't a bug... what? When something is not acting as expected is that not the definition of a bug? ( Or are you saying this is currently as intended. What, what?) :)

I can understand you not wanting to fix something right away that can majorly impact the game behavior, expected or not. If you are going to implement a feature, make sure it is doing what you want. That may take some thought. But lets still call a spade a spade. :)

Just to clarify a bit about Matt's "bug" comment - I think what he means is that we are focusing more on game breaking and stability issues that might cause, for example, the game to crash or maybe the save game gets corrupted and gameplay can't continue at all. There are also some smaller bugs that are getting fixed as well, if they are easy to implement and test.

Anyway, thank you for bringing up this spotting issue. We can all agree that it has room for improvement.

Matt-Caspermeyer commented 7 months ago

I'm pleased to announce that due to tester feedback, we've decided to move up the implementation of this fix and put it into the latest release.

If you would like to be a part of the group that will test this change, please contact either @HDIAndrew or @Xenotrenium in the EFS Discord channel.

Xenotrenium commented 7 months ago

This is something we'd call a "Flaw" or "Defect" and not a "Bug". It is indeed semantics, but indeed the end result is unintended behavior. We're going to initiate playtesting of this new system shortly - this feedback is part of what pushed us to change it. Thanks @Mastnosis ! :)

Mastnosis commented 7 months ago

I'm curious to know the results of the changes. Did it noticeably alter game behavior? Was there an improvement in gameplay?