Allow configuration of unit view distance#41
Conversation
…or common dist check
… fortification bonuses
|
Being clearer on my commenting, someone with the ready ability to check that game version would be helpful. The patches are all in locations calling can_see_tile and the update_visibility function |
maxpetul
left a comment
There was a problem hiding this comment.
Apologies for taking so long to have a look at this but I've just been busy with so many other things. It's good work. I was worried that this feature would be gnarly but this is a nice clean implementation. The changes I've requested are just small things. Also I've fixed the PCGames.de addresses and verified that it works on that EXE.
|
No worries on the wait, there's a lot of stuff and I can fully understand taking time for a hobby. |
|
Looks good except for two things. First, with the default rule Second, when I mentioned dropping "UTC_" from the Land/Sea/Air keywords in the config, I also had in mind that you would make it so that the keywords only applied if they were not in quotes. I.e., |
…ed unit class in configs.
|
Okay, found the energy to actually look at the fortification code. Yes, it was unintentionally stacking with the flat bonus because the total sight distance was being recalculated, which I don't consider to be intentional. I've patched that and the config naming. I would say the config of 2 is, although initially untuitive, correct. The mechanic works regardless of terrain or sightlines just like radar, so re-including terrain in its calculation is wrong. Relying on the flat bonus would incorrectly mean a tile at distance 3 obscured by land would not be seen when it should be. Arguably if I spent more time I could configure fortification options to have more customizable stacking behaviour. Though then we're also looking at land height bonuses which is a realm the vanilla game doesn't touch. |
Alright, that makes sense. I'd consider this PR finished and ready to merge at this point. There's just one very minor issue left which I was reminded of looking at your changes to the config: the grammar mistakes in "base visibility can be give... bonus visibility can be give", but I can fix that no problem. I'll merge this after PR #31 which should be very soon. |
|
Bruh |
Adds configuration of unit visibility ranges.
Adds configuration of terrain visibility behaviours.
Supports fortification bonuses, euclidean radii, the sea being flat, and all the bells and whistles.
The default configuration should match vanilla, but on a basis of caution everything is toggled under 1 flag. There is also a performance reason for this, as at a high visibility with hundreds of units around this can become a taxing system.
Resolves #25
This was tested on the Steam version of the game.
Exe offsets should be correct for GoG as there is where most RE took place, but a test should be performed.
OFFSETS FOR PCG.DE ARE UNCHECKED, AS IS THE GAME.
UPDATE BEFORE MERGE.