Add settler tile adjustments to Civilization class #539
Conversation
- Add `SettlerTileAdjustments` as a child of `Civilization`
- Encodes the previously hard-coded values as defaults for public fields
- Uses lambda functions to take values at runtime for adjustments that take
arguments; this allows future users to supply arbitrary logic
- Replace hard-coded bonus values in `SettlerLocationAI.cs` with calls and
references to `SettlerTileAdjustments` via the supplied `Player`
- Notably, `DistancePenalty` is now a negative value by default, which I
think is more clear because now adjustments are always added to the tile
score
- Make settler tile score a `float` instead of `int`, which gives more
precision for multiplicative adjustments
- Apply lots of lints from Rider
- Identifier case tweaks (this caused most of the changes in other files)
- Using ternary operators to make code a little smaller when it's still clear
- Some grammar problems (Rider is picky about these for some reason)
- Lint ignores in cases where it makes sense (to me)
- I'm willing to undo these if they're too aggressive or unwanted
TomWerner
left a comment
TomWerner
left a comment
There was a problem hiding this comment.
Thanks for the PR, and the detailed description!
Would it be possible to split these apart into a couple of PRs? In particular, I think a PR that only fixes linting issues would be ideal. The linting issues touch so many files that its hard to see the actually interesting changes, so reducing diff noise would make it much easier to review the PR confidently.
Also it looks like C7/C7.sln.DotSettings is a new file, was that intended to be included in this PR?
Makes sense to me. I'll pull out the lint changes into a new PR and then go through the (many) other ones on that separate branch. It will take some time but will make for a cleaner development experience because the UI will quiet down. Only the parts that are directly related to #92 will remain here.
This is part of the linting changes. Rider applies spelling and grammar linting in comments 🙄 and some of the words that appear in comments aren't in its dictionary (e.g., "civilopedia"). It's possible to add the unrecognized words to a dictionary so Rider stops flagging them, either locally (untracked) or "team-shared" (tracked). The file you mentioned is where Rider stores this configuration. I think tracking this file is a good idea because it should prevent this kind of linter noise in all Rider installations for this project. A better option would probably be to find a way to disable this kind of linting entirely, but that should be a discussion elsewhere. I hadn't used Rider before this project, so I'm sure there's a lot I don't know here. Do you have any thoughts about my approach related to #92? I'm still unfamiliar with the project internals and C# in general, so it wouldn't surprise me if it can be improved. |
If you're talking about the changes to |
|
Linting changes removed. This PR now only contains the changes related to the settler AI. Ready for another pass @TomWerner I'm not sure whether I'm running the tests right, but on my system there are 12 of 46 failing on this branch, compared to 8 failing on |
2 participants
SettlerTileAdjustmentsas a child ofCivilizationSettlerLocationAI.cswith calls and references toSettlerTileAdjustmentsvia the suppliedPlayerDistancePenaltyis now a negative value by default, which I think is more clear because now adjustments are always added to the tile scorefloatinstead ofint, which gives more precision for multiplicative adjustmentsApply lots of lints from RiderIdentifier case tweaks (this caused most of the changes in other files)Using ternary operators to make code a little smaller when it's still clearSome grammar problems (Rider is picky about these for some reason)Lint ignores in cases where it makes sense (to me)I'm willing to undo these if they're too aggressive or unwantedCloses #92