|
| 1 | +# TODO |
| 2 | + |
| 3 | +Issues noticed while writing the documentation in `docs/`. None of them has been fixed yet; each |
| 4 | +needs review by the code owner before changing. |
| 5 | + |
| 6 | +## Possible bugs |
| 7 | + |
| 8 | +- [ ] `core/e2core/Localize.cpp:213` — `RemoveTokensFile` calls `m_tokens.remove(it)` and then |
| 9 | + `++it` on the removed iterator (use after free). `Map::remove` returns the next iterator; the |
| 10 | + loop should continue from it. |
| 11 | +- [ ] `core/e2core/Localize.cpp:302` — `AddToken(const char*, const wchar_t*)` passes the wide |
| 12 | + string to `LocalizeConvertSymbols` through `(char*)`. The function parses it as a narrow string |
| 13 | + and stops on the second byte of the first character, so `\n` / `\t` are not converted for this |
| 14 | + overload. |
| 15 | +- [ ] `core/e2core/Localize.cpp:302`, `:314` — both `AddToken` overloads cast away `const` and |
| 16 | + modify the caller's string in place (undefined behavior for literals). |
| 17 | +- [ ] `core/e2core/Localize.cpp:160` — when `RegionalSettings` is missing, the message says |
| 18 | + "force english" but `m_language` stays empty and the `$LOCALIZE$` search path is already removed. |
| 19 | +- [ ] `public/materialsystem1/IMaterialSystem.h:235` — template `SetupDrawDefaultUP(topology, |
| 20 | + const VERT* verts, ...)` passes `&verts` (address of the local pointer) instead of `verts`. |
| 21 | +- [ ] `public/render/IDebugOverlay.h:62-63` — `DDNodeBase::nameHash` and `frameindex` are not |
| 22 | + initialized. Without `Name()` the primitive may be treated as named by |
| 23 | + `CDebugOverlay::CheckNodeLifetime` (`nameHash != 0`) and replaced by an unrelated primitive. |
| 24 | +- [ ] `framework/core/platform/eqthread.h:101`, `:162` — `CScopedMutex` and `CScopedWriteLocker` |
| 25 | + with `blocking = false` unlock in the destructor even if the lock was not acquired. |
| 26 | +- [ ] `framework/core/platform/eqatomic.h:62` — `Atomic::CompareExchangeWeak` takes `oldVal` by |
| 27 | + value, so the updated expected value is not returned to the caller. |
| 28 | +- [ ] `framework/core/platform/eqjobmanager.cpp:260` — `CEqJobManager::AllJobsCompleted` dequeues a |
| 29 | + job and enqueues it back to check the queue, which changes the queue order (and is racy with |
| 30 | + workers). |
| 31 | +- [ ] `framework/ds/Map.h:168`, `:499` — `clear()` and `remove()` read `item->next` after calling |
| 32 | + `item->~Item()`. Works in practice, but is undefined behavior; read `next` before destroying. |
| 33 | + |
| 34 | +- [ ] `shared/font/FontCache.cpp:253` — `GetFamily` returns the default family for unknown names, |
| 35 | + so `GetFont(..., defaultIfNotFound = false)` never returns `nullptr` once a default font exists. |
| 36 | + |
| 37 | +## Code quality |
| 38 | + |
| 39 | +- [ ] `shared/font/FontCache.cpp:47` — font description loading uses the deprecated |
| 40 | + `KV_LoadFromFile`. |
| 41 | +- [ ] `shared/studio/StudioGeom.cpp:708` — `.skin` loading uses the deprecated `KV_LoadFromFile`. |
| 42 | +- [ ] `tools/fcompress/fcompress_main.cpp:390`, `:520` — config and KeyValues conversion use the |
| 43 | + deprecated `KV_LoadFromFile` / `KV_GetValue*`. |
| 44 | +- [ ] `public/egf/*.h`, `shared/egf`, tools — headers and comments still call EGF "Equilibrium |
| 45 | + Geometry Format" / "Equilibrium Graphics File"; the format is now named Entity Geometry Format. |
| 46 | + |
| 47 | +- [ ] `framework/core/ConCommand.h:17` — `CMD_ARG_CONCAT` uses an undefined `i` and appends the |
| 48 | + separator once after the loop instead of between arguments. Appears unused. |
| 49 | +- [ ] `core/e2core/profiler_core.cpp:158` — `ptrace_stop` description is "Performance trace start" |
| 50 | + (copied from `ptrace_start`). |
| 51 | +- [ ] `framework/core/platform/eqthread.h:222`, `:237` — the usage comment mentions a non-existent |
| 52 | + `StartWorker()` and its worker example calls `StartThread` instead of `StartWorkerThread`. |
| 53 | +- [ ] `public/core/IEqParallelJobs.h:27` — comment mentions `JOB_FLAG_DELETE`, which does not exist |
| 54 | + (the job is created with `DeleteOnFinish`). |
| 55 | +- [ ] `framework/math/Utility.cpp:36`, `:50` — `PointToScreen` returns `true` when the point is |
| 56 | + **behind** the camera; the name suggests the opposite. Consider renaming the result or |
| 57 | + documenting it in `Utility.h`. |
| 58 | +- [ ] `core/e2core/Localize.cpp:113` — `Init` lower-cases `m_language` again after |
| 59 | + `SetLanguageName` already did it. |
| 60 | +- [ ] `core/e2core/Localize.cpp` — style does not match `docs/codestyle.md` (C-style casts, |
| 61 | + `_FindToken` reserved identifier, missing `override`, `pszDefaultToken = 0`, mixed `if(` / |
| 62 | + `if (`). Only relevant if the file is refactored. |
| 63 | + |
| 64 | +## Features |
| 65 | + |
| 66 | +- [ ] `tools/egfca` — generate `.skin` files (additional model skins) from the compile script, so |
| 67 | + they do not have to be written by hand. |
| 68 | + |
| 69 | +## Build scripts |
| 70 | + |
| 71 | +- [ ] `shared/premake5.lua:189` — `sysLib` lists `gamesys/**`, which does not exist in `shared/`. |
| 72 | +- [ ] `samples/GPUDriven/premake5.lua:26` — uses `egfLib`, which is defined only when |
| 73 | + `ENABLE_TOOLS` is on. |
| 74 | +- [ ] `utils/.premake_modules/usage` — the custom `usage` / `uses` module conflicts with the |
| 75 | + built-in usages of newer premake5 releases (they work differently), so the build is pinned to |
| 76 | + the premake5 binaries in `utils/`. Resolve the conflict (rename the module API or migrate to |
| 77 | + the built-in usages) to allow updating premake5. |
| 78 | + |
| 79 | +## Documentation to verify |
| 80 | + |
| 81 | +- [ ] `docs/keyvalues.md` — V3 array elements separated by whitespace (`[bottom hcenter]`) and the |
| 82 | + one-line `embed1 { embeddedFlags flag1 flag3; }` form in V2 were not verified against the parser. |
| 83 | +- [ ] `docs/public_filesystem.md` — whether `CFileSystemFind::GetPath()` returns a full path or only |
| 84 | + the entry name. |
| 85 | +- [ ] `docs/framework_math.md` — `QuadTree.h` description (Barnes-Hut style) is inferred from method |
| 86 | + names. |
0 commit comments