17
0
mirror of https://github.com/MaSzyna-EU07/maszyna.git synced 2026-11-08 11:34:19 +01:00
Files
maszyna/CLAUDE.md
maj00r 939e42b611 docs: CLAUDE.md with the team's notes for Claude Code (#258)
* docs: CLAUDE.md with the team's notes for Claude Code

The rules Claude Code sessions kept relearning, shared with the team:
- the code builds and behaves the same with GCC 13 (Linux CI and the
  Sonar build) as with MSVC, with the MSVC leniencies to watch for;
- every SonarCloud remark on the changed code gets fixed without
  changing behaviour, with the recipes for the rules that keep coming
  up;
- building, the unit tests, scripted editor runs, the game data and
  the profile folders, and GPU hangs;
- the game data in the asset SVN and backward compatibility: existing
  files keep being read as before, new entries are optional, code
  falls back without new assets and a change that uses them says so
  in its commit message, what the editor saves stays readable by the
  current release, new features can be turned off in eu07.ini and
  don't change the look of existing data abruptly; StarterNG is the
  official starter.

* docs: CLAUDE.md on the move to OpenGL 4.6 and Vulkan

betterRenderer/ is to be removed and the OpenGL 3.3 and legacy
renderers are being replaced by feat/gl46-migration (PR #216, a draft
with milestone 27.04). Work on that code and on the shaders is kept to
fixes and to what a feature on master needs to be drawn, with the PR
saying where the branch has the counterpart.

---------

Co-authored-by: maj00r <maj00r@host>
2026-11-07 16:17:57 +01:00

164 lines
13 KiB
Markdown

# MaSzyna: notes for Claude Code
## The code builds and works with GCC, not only MSVC
The Linux CI (`.github/workflows/build.yml`) builds with GCC 13 on ubuntu-24.04, through the Sonar build-wrapper, and `sonar.yml`
analyses that build once it has passed. A change that compiles with MSVC only fails there, and a failed GCC build also leaves Sonar
without an analysis of the PR. So every change has to compile with GCC, and behave the same with it, as well as with MSVC. The CI
builds with `-DWITH_BETTER_RENDERER=OFF` and Sonar excludes `betterRenderer/`, so that part is neither built on CI nor analysed.
Where no GCC is at hand, read the new code for what MSVC lets through and GCC doesn't:
- `constexpr` initialised from a floating or class-type constant declared only `const`: make the source `constexpr` too.
- Standard headers MSVC pulls in through others: include them yourself. For example, `<ranges>` for `std::views`, `<format>`,
`<numbers>`, `<span>`, `<cstring>`, `<cfloat>` and `<bit>`.
- `typename` and `this->` where a template needs them.
- A temporary bound to a non-const reference.
- Explicit specialisations inside a class.
- A name a class uses and then declares again in its own body (GCC: "changes meaning").
- A nested type with default member initialisers used as a default argument inside the class that encloses it.
- A declaration changed in a header (`const`, parameters) without its definition, or a new member function defined but not declared.
- `std::ifstream` / `std::ofstream` opened with a `std::wstring`.
- MSVC-only functions: `_stricmp`, `sprintf_s`, `fopen_s`, `localtime_s` and the like.
- Designated initialisers out of declaration order.
- `emplace_back(a, b, c)` with parenthesised aggregate initialisation is fine on GCC 13, but not for `std::array`. There is no brace
elision in parentheses, so build a named local instead.
The same behaviour needs portable code. Don't rely on the order arguments are evaluated in, on the signedness of `char`, on the size of
`wchar_t`, or on `long` being 32 bits.
## SonarCloud: every remark on the changed code gets fixed
SonarCloud (`sonar.yml`, `sonar-project.properties`) analyses each PR. A PR counts as finished only when every remark Sonar raises on
the code it changes is fixed. That means the code smells too, not just the bugs, the vulnerabilities and the quality gate. The analysis
runs on the GCC build, so a PR that fails to build with GCC gets no analysis at all.
Fixing a remark must not change what the code does:
- Keep the order of side effects, the conditions and the floating-point results as they were.
- Text in `STR(...)`, `STR_C(...)` and `_(...)` is looked up in the translations (`lang/*.po`), so it stays byte for byte the same.
The same goes for what is written to the log.
- `// NOSONAR: <reason>` is only for a real false positive, or where a fix would change behaviour. Use it rarely, always with the
reason.
Write new code so that it doesn't raise the rules that keep coming up:
- **S134** (nesting deeper than 3 `if` / `for` / `while` / `switch` / `try`) and **S3776** (cognitive complexity over 25):
- use early `continue` / `return`;
- move loop bodies and branches into named helpers, preferably free functions in the anonymous namespace;
- group the state of a long algorithm in a small struct with methods.
Every `else if`, every `&&` / `||` sequence and every nested lambda adds to the complexity.
- **S1188**: a lambda over 20 lines becomes a named function.
- **S107**: a function with more than 7 parameters takes a struct.
- **S3608**: lambdas capture explicitly (`[this, &points]`, not `[&]` or `[=]`). The only exception is `[&]` on a lambda passed
straight to a standard algorithm; `[=]` is flagged even there, and so is `[&]` passed to one of our own functions.
- **S6197** / **S5566**: use the `std::ranges::` algorithm that takes the container rather than a begin/end pair or a hand-written
loop.
- **S6165**: use `std::erase_if` rather than erase-remove.
- **S6004**: a variable used only by the condition is declared in the `if (init; condition)`.
- **S3358**: no nested `?:`; use an `if` or a small function.
- **S1121**: no assignment inside an expression, chained ones included (`a = b = c`).
- **S1117**: no declaration shadowing a local, a member or an enumerator.
- **S6003**: `v.emplace_back(a, b, c)` rather than `v.push_back({a, b, c})`. For a `std::array` element, push a named local instead
(see the GCC notes above).
- **S6012**: let class template argument deduction work: `std::vector values{1.0, 2.0}`.
- **S5945**: `std::array` / `std::vector` / `std::string`, not C arrays.
- **S1659**: one declaration per statement.
- **S6009**: `std::string_view` rather than `std::string const &` for a string parameter that is only read.
- **S6185** / **S6494**: `std::format` rather than concatenation or `snprintf`.
- **S924**: at most one `break` per loop.
- **S886**: a `for` whose counter changes in its body becomes a `while`.
- **S5997**: `std::scoped_lock` rather than `std::lock_guard`.
- **S6168**: `std::jthread` rather than `std::thread`. A `std::jthread` joins in its destructor where a `std::thread` would
terminate, so in existing code switch only where the thread is always joined or detached before it is destroyed.
- **S6177**: `using enum` where a function names many enumerators.
- **S5350** / **S995**: `const` on references and pointers that aren't written through.
- **S5817**: `const` on member functions that change nothing.
- **S2193**: no `float` loop counters.
- **S5421**: no non-const globals.
- **S6179**: `std::lerp` rather than a hand-written interpolation, in new code. In existing code only where it gives the very same
result.
- **S6164**: `std::numbers::pi` rather than a literal, in new code. In existing code only where it gives the very same result.
The project is C++20 (`CMAKE_CXX_STANDARD 20`), so no C++23 library addition is available on any compiler. That rules out
`std::ranges::to`, `views::zip`, `views::enumerate`, `ranges::contains`, `std::print` and `std::expected`.
## Renderers: the move to OpenGL 4.6 and Vulkan
The renderers on `master` are to be replaced. `feat/gl46-migration` (PR #216, a draft with milestone 27.04, to be merged after the
26.11 release) replaces the OpenGL 3.3 renderer with an OpenGL 4.6 core one and adds a Vulkan 1.4 renderer beside it. It also
removes the legacy OpenGL 2.1, the OpenGL ES / ANGLE and the NVRHI (`betterRenderer/`) paths.
- `betterRenderer/` will be removed. Don't put work into it.
- On that branch, of the `rendering/opengl*` files only `openglcamera.*` and `openglmatrixstack.*` stay. The whole
`rendering/opengl33*` family and the rest of the legacy `rendering/opengl*` go, and the new renderers are in
`rendering/opengl46/`, `rendering/vulkan14/` and `rendering/common/`. The shared `rendering/renderer.*`, `geometrybank.*`,
`particles.*` and the like stay. The shaders move from `shaders/` to `shaders/ogl46/` and `shaders/vk14/`.
- So a change on `master` to the code that goes away, or to the shaders, may be pointless. Keep it to fixes and to what a feature
on `master` needs in order to be drawn at all. For such a change, say in the PR where `feat/gl46-migration` has the counterpart,
or that the branch already carries the change.
## Building, testing and running
- The unit tests (`tests/unit`) run at the end of the build, which prints `Unit tests: N/N passed` and fails on any failed case.
`-DWITH_TESTS=OFF` builds the game without them.
- A new feature goes on its own branch from `master`, not on whatever branch is checked out. Say which branch the work lands on
before the first edit. A fresh worktree has empty submodules, so it doesn't build until they are filled.
- The editor runs a script when `EU07_EDITOR_SELFTEST=<file>` is set: one command per line, `#` for comments. The commands are in
`application/editormodeselftest*.cpp` (`camera`, `wait`, `select`, `lay`, `screenshot`, `save`, `quit` and many more).
- Run it on a copy of the scenery, never on the original, and delete the copy afterwards.
- Give every run a timeout and keep an eye on its memory. A loop gone wrong can fill the RAM of the whole machine.
- `screenshot` only queues the grab and waits 3 frames; the image is written later on another thread. A click right after it
has been seen to lose the selection, so leave a `wait` of about 15 frames after a `screenshot` before clicking.
- At run time the editor's texts load from `lang/editor_<language>.po` in the game folder (`utilities/translation.cpp`), not from
the build. New strings of a branch show in English until that file has them. Without the `fonts` folder Polish letters show as `?`.
- When a build runs on a copy of the game data made of links to the real folders, turn off whatever writes into the data
(`convertmodels 0` in `eu07.ini`), or the run changes the real game folder through the links.
- A test of code that moves, migrates or deletes files in the user's profile (`%APPDATA%`, `%LOCALAPPDATA%`, caches) gets its own
test folder. Never point it at the real profile.
- A GPU hang (Windows TDR) can freeze the whole machine, not just the game. Don't test a hang hypothesis by running it. Guard the
code against what causes it (degenerate geometry, NaN in the camera or in transforms) and read the `LiveKernelEvent` / watchdog
dumps instead.
- PowerShell variable names are case-insensitive: `$Track` and `$track` are the same variable.
## Game data and backward compatibility
The code and the game data live apart. The assets (sceneries, vehicles with their `.fiz` and `.mmd`, models, textures, sounds,
timetables, most of `lang` and the shipped `eu07.ini`) are in a separate SVN repository, `https://svn.eu07.pl/pctga`. It needs an
account; there is no anonymous checkout. Of `lang/`, only `editor_pl.po` is in this repository. Players install and
launch the game through Steam, GOG and the like, and run new builds on the data they already have.
The official starter is StarterNG (`https://github.com/MaSzyna-EU07/StarterNG`), a separate repository; the older
`MaSzyna-EU07/Starter` is not the one in use. `launcher/` in this repository is the simulator's built-in scenario and vehicle
picker (`launcher_mode`), not StarterNG.
- Every existing file must still be read the way it was. Don't rename or drop a keyword, change the meaning or the unit of a value,
or make an optional entry required. This holds for every format the game reads: for example `.scn` and its includes, `.fiz`,
`.mmd`, `.e3d` / `.t3d`, `.mat`, the `.txt` timetables and track profiles, `eu07.ini` and the Python screens.
- A bug fix may change what an existing file leads to, such as how a vehicle from its `.fiz` behaves, as long as the PR says so.
- Something new in a format is optional, and without it the code does exactly what it did before. A file that loaded correctly
before doesn't get new warnings or errors in the log. A real fault that is newly detected may be logged.
- A code change must not need a change in the SVN to work. Where it uses new or changed assets, it falls back to the old behaviour
without them, and the PR says what is to be added to the SVN and in what order the two go in.
- A change that uses new or changed assets (and falls back without them, see above) says so plainly in its commit message: which
files, and what is to change in them. It is never left implied. Before such a commit, either make the asset change right away in
the user's SVN working copy, or get the user's agreement to leave it for later. Ask the user where their SVN working copy is; the
installed game folder is not one. Pushing to git and committing to the SVN both need the user's consent.
- A scenery saved by the editor without using a new feature holds nothing the current release can't read. The entries of a new
feature (such as a new kind of node) appear only where that feature was used, and the PR says that the release doesn't read
them.
- Saving patches the scenery files by byte spans: unchanged definitions stay byte for byte, and an edited node is written anew by
its `export_as_text`. A change to saving must not widen what gets rewritten, nor make `export_as_text` drop more of an edited
node (comments, unknown tokens) than it did before.
- A new feature that changes what the simulator does or shows with existing sceneries and vehicles can be turned off in
`eu07.ini`, and turned off, the simulator behaves as it did before the feature. Editor tools and new optional entries of a
format are covered by the points above. The key is parsed in `utilities/Globals.cpp` (`global_settings::ConfigParse*`) and
written by `global_settings::export_as_text`, which puts the settings in the log at start. `eu07.ini` is read from the user's
profile (`user_config_path`), and from the game folder only when the profile has none; test the switch in the file that is
actually read.
- A feature doesn't make existing sceneries and vehicles look suddenly different after an update. A change to how things look is
something the player turns on, or comes in gradually.
- When a format really has to change, read both the old and the new form, and say in the PR what changes for the data and for
the people who make it.