-
Notifications
You must be signed in to change notification settings - Fork 16
Coding style
Code is formatted with cargo fmt at the default settings except that use_small_heuristics = "Max" is set to make better use of the screen space.
The rustfmt.toml file in the repo root directory takes care of this.
Some of the huge tables could probably benefit from an exception to this formatting, to keep their rows as single lines, but that hasn't been done yet.
The code is intended to be viewed with a 130 character wide terminal, and it's okay to use that full width (within the limitations of cargo fmt).
Multiline comments should generally be less wide, though, simply for readability.
Short-term variables are often just named after their type or role. If you have a key and its block, just call them "key" and "block"; no need to get creative.
A Validator is always called "vd", a ScopeContext is always called "sc", the &Everything reference is always called "data", etc.
Using consistent names for each type, and only using other names when it really matters, will lighten the cognitive load for the reader once they are familiar with these conventions.
Function arguments are in a fairly consistent order: key, block or bv, data, sc, vd where applicable, followed by any booleans, with closures at the end.
Lookup functions take an Item and then a &str or Token.
Validator functions start with the field name.
(These last two rules sometimes conflict, giving us vd.field_item(name, item) and data.verify_exists(item, name)).
Making a full validator for multiple games is a huge job, and it's more important to get it working than to make it complete.
For that reason, don't be afraid to make something and add // TODO comments for everything that still needs more work.
We will get to all the TODO comments one day before the heat death of the universe.
Many TODO comments require some research, such as verifying how something works in-game. Try to add the word "verify" to each such comment, so that they can be easily searched for by people who want to contribute something other than code.
Because we have a separate developer working on Imperator: Rome validation, it's best to start TODO comments for that game with // TODO - imperator -.
The validator defines a feature for each game it supports (ck3, vic3 and imperator).
Normal cargo rules are that features must be additive, not conflicting, but we're breaking that rule.
You can turn on either ck3 or vic3 or imperator but if you turn on more than one at a time... it will compile, but probably not validate correctly.
The feature is selected by deciding whether to build the ck3-tiger or the vic3-tiger or the imperator-tiger crate.
Each contains a binary and depends on the feature it needs.
The code should be buildable with each feature without emitting warnings. This can sometimes involve silencing warnings with #[allow(...)] attributes or #[cfg(feature...)] attributes in the code. If you apply a #[cfg(feature...)] only because some code is unused in other games (not because it's not intended for those other games) then please mark it with a comment saying so.
NOTE: Making it possible to turn on more than one feature at a time is a recent development, done in order to make "cargo doc" work.
It means you can no longer rely on the features to decide which game's logic to apply.
To support correct operation when there is more than one feature defined, there is the Game type, with some functions to query which game is being validated.
You will often have to do both a #[cfg(feature = "X")] check and a Game::is_X check to make some game-specific code both compile and run correctly.
The long term goal is to make it possible to have a single tiger-lib library with all the features turned on, which can validate any game on demand. (But no more than one game per run; changing that will have severe performance impacts).
use statements go at the top of the file, with imports from std in a separate block at the top, then imports from other crates, then crate:: imports from the validator itself. If any enum variants are imported in bulk, they go in a block below that. (Exception: it's okay to put Eq::* as part of the crate::block imports. It just looks better that way; I don't know why.)
Each block is sorted alphabetically.
Multiple items are combined into { lists } only when they come from the same module, with the exception of data modules imported into the everything module. They are combined to avoid repeating the #[cfg(feature = ...)] over and over.
Importing enum variants in bulk should be rare; it's generally done to make large table declarations smaller, with those tables being in modules just for the tables.
While "cargo clippy" is used as a guide, it is not necessary to fix everything it warns about.
Do silence such warnings with #[allow(clippy:...)] though.
Clippy should be silent for every supported game, so that new warnings are easy to spot.
When fixing clippy warnings, keep in mind that there are cases where clippy gives bad advice. Feel free to ignore those. It's helpful to add a comment explaining why you're not fixing it, or a TODO comment if you do intend to fix it someday.