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
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.
| ; There are two mutually-exclusive types of fortification bonus: when_fortified and when_fortified_same_continent. The latter only applies to tiles on the same continent. | ||
| ; The first rule matching a unit is used. Therefore, unit class rules should be at the end. | ||
| ; The following is an example: [Scout: 1 + 2 times_bonus, "Landmine": 0 + 0 times_bonus, "Army": 2 + 2 when_fortified] | ||
| unit_visibility_rules = ["UTC_Sea": 2 when_fortified_same_continent] |
There was a problem hiding this comment.
I'd like you to change the format here a bit. First, when_fortified_same_continent should use hyphens instead like when-fortified-same-continent, same for the other keywords. Using hyphens instead of underscores here is arbitrary, but it's what I've been doing for all the other config keywords and I want to keep it consistent.
Second, I think it's a bit ugly to leak "UTC_" into an area users will see it. That prefix makes sense given that those values are part of an enum but it's strange without that context. I know the point of including the prefixes is so that the unit class names are less likely to conflict with actual type names, but I'd rather that be handled with quotation marks, as in e.g. Sea is a special word referring to naval units and "Sea" refers to a unit type named Sea. Come to think of it, sea should be accepted too as config keywords are generally not capitalized.
Edit: Also unit_visibility_rules should be 1 when_fortified_same_continent by default to match the vanilla rules. This setting here annoyed me when testing because I was trying to set rules for UTC_Sea or Galley units in a scenario config and they weren't having any effect. I thought it was a bug at first before I remembered about this setting in the default config. It gets loaded first and overrides any settings for sea units in the scenario config. I think it's better to search for matching rules in reverse order so what's specified last takes precedence. That would be consistent with how other config options work.
There was a problem hiding this comment.
hyphens you are correct
I can handle mapping names to avoid UTC sure
reading in reverse order would make sense. it's a bit silly that these are grouped into the same list but it just ended up cleaner that way. Sea bonus itself is a bit odd because of the bonus distance over water tiles. Might need to re-check what matches vanilla but I recall the slightly unintuitive value because of this.
|
No worries on the wait, there's a lot of stuff and I can fully understand taking time for a hobby. |
2 participants
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.