Skip to content

Commit abbc8c5

Browse files
GWToolbox Botclaude
andcommitted
style(account-inventory): one-line comments per AGENTS.md
Collapse the prose-block comments added in this PR to single why-lines and use auto for TIMER_INIT()/captured locals, per the repo comment/variable guidelines. No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 30c66d6 commit abbc8c5

1 file changed

Lines changed: 17 additions & 40 deletions

File tree

GWToolboxdll/Windows/AccountInventoryWindow.cpp

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,19 +1010,12 @@ namespace {
10101010
bool success = false;
10111011
};
10121012

1013-
// One dispatched load. A worker thread fills `results` and sets `done`; the main
1014-
// thread applies them (in Update, or a blocking flush) once done. Held behind a
1015-
// shared_ptr so an in-flight worker never dangles if the module is torn down or a
1016-
// newer load supersedes this one mid-parse.
1013+
// Dispatched load: a worker fills results + sets done, the main thread applies it once done.
10171014
struct PendingLoad {
10181015
std::vector<LoadResult> results;
1019-
std::atomic<bool> done{false};
1016+
std::atomic<bool> done{false}; // shared_ptr-owned, so a superseded/torn-down worker orphans safely
10201017
};
1021-
// Serialises file access only: a worker holds it around each file read, SyncAccountFile
1022-
// around each write/delete, so a save can't rewrite/delete a file mid-read. Parsing and
1023-
// JSON serialisation run outside it. (Completion is signalled by PendingLoad::done, not
1024-
// this mutex.)
1025-
std::mutex load_mutex;
1018+
std::mutex load_mutex; // guards file access only (worker read vs SyncAccountFile write); done signals completion
10261019
std::shared_ptr<PendingLoad> active_load;
10271020

10281021
// Encode the wide game string as fixed-width 4-hex code units with no separators
@@ -1114,8 +1107,7 @@ namespace {
11141107
item.equipped = j.equipped.value_or(0);
11151108
const std::wstring enc = DecodeDescription(j.description);
11161109
item.description.reset(enc.c_str()); // EncString decodes lazily for display
1117-
// texture is fetched on demand in Draw (see below) so loading files doesn't flood
1118-
// the worker pool with an icon decode per stored item, most of which are never shown.
1110+
// texture fetched on demand in Draw, so loading doesn't decode an icon per stored item
11191111
}
11201112

11211113
void ApplyFreeSlots(FreeSlotInfo& fs, const FreeSlotsJson& j)
@@ -1158,8 +1150,7 @@ namespace {
11581150
return true;
11591151
}
11601152

1161-
// Worker-thread: parse one already-read account file into a LoadResult. Pure CPU on local
1162-
// memory - no file access, no hierarchy access - so it runs outside load_mutex.
1153+
// Worker-thread: parse one already-read account file; pure local work, runs outside load_mutex.
11631154
LoadResult ParseIniFile(const std::filesystem::path& path, const std::string& contents, bool only_foreign, const GUID& account)
11641155
{
11651156
LoadResult r;
@@ -1227,20 +1218,16 @@ namespace {
12271218
return;
12281219
}
12291220

1230-
// Reading + parsing the files off the game thread. A newer load simply replaces
1231-
// active_load; any superseded worker keeps its own PendingLoad alive and finishes
1232-
// harmlessly into an orphan.
1233-
const GUID captured_account = current_account;
1221+
// parse off the game thread; a newer load replaces active_load and orphans this one
1222+
const auto captured_account = current_account;
12341223
auto pending = std::make_shared<PendingLoad>();
12351224
active_load = pending;
12361225
Resources::EnqueueWorkerTask([pending, to_load, only_foreign, captured_account] {
12371226
for (const auto& path : to_load) {
12381227
std::string contents;
12391228
bool read_ok;
12401229
{
1241-
// Only the file read races a concurrent save/delete; the parse below is
1242-
// pure local work, so the lock is held for the read alone.
1243-
std::scoped_lock lock(load_mutex);
1230+
std::scoped_lock lock(load_mutex); // only the read races a save/delete; parse is unlocked
12441231
read_ok = ReadFileToString(path, contents);
12451232
}
12461233
if (!read_ok) {
@@ -1254,18 +1241,14 @@ namespace {
12541241
pending->done.store(true, std::memory_order_release);
12551242
});
12561243

1257-
// A foreign-only reload (map/character change, module toggle) is applied later in
1258-
// Update(), so it never stalls the game thread - this is the load-in cost we're
1259-
// removing. A full load feeds the current account's own stored history that
1260-
// Initialize()/Delete All immediately build on, so it must complete before we return.
1244+
// foreign reload applies later in Update (no stall); a full load must finish now, callers build on it
12611245
if (only_foreign)
12621246
needs_sorting = true;
12631247
else
12641248
FlushPendingLoad();
12651249
}
12661250

1267-
// Apply a finished load's parsed results into the hierarchy (main thread). The caller
1268-
// must have observed active_load->done, so the worker is no longer touching results.
1251+
// Apply a finished load into the hierarchy (main thread); caller must have observed done.
12691252
void ApplyPendingLoad()
12701253
{
12711254
if (!active_load) return;
@@ -1275,24 +1258,20 @@ namespace {
12751258
needs_sorting = true;
12761259
}
12771260

1278-
// Non-blocking: apply the in-flight load if its worker has finished. Called every Update.
1261+
// Called from Update: apply a finished load without blocking the game thread.
12791262
void DrainPendingLoad()
12801263
{
12811264
if (active_load && active_load->done.load(std::memory_order_acquire))
12821265
ApplyPendingLoad();
12831266
}
12841267

1285-
// Blocking: wait for the in-flight load to finish, then apply it. Used before tearing
1286-
// the module down or before a save that may delete/overwrite files a worker is reading.
1268+
// Blocking: wait for the load to finish then apply it (before teardown / a mass file rewrite).
12871269
void FlushPendingLoad()
12881270
{
12891271
const auto pending = active_load;
12901272
if (!pending) return;
1291-
const clock_t start = TIMER_INIT();
1292-
// The worker sets `done` only after its last read, so waiting on it means no file
1293-
// access is left in flight (the read/write races are handled by load_mutex directly).
1294-
// The worker may still be queued behind other pool work; wait for it to publish. If it
1295-
// never ran (pool starved past the timeout) done stays false and a later Update applies it.
1273+
const auto start = TIMER_INIT();
1274+
// if the pool starved the worker past the timeout, done stays false and a later Update applies it
12961275
while (!pending->done.load(std::memory_order_acquire) && TIMER_DIFF(start) < 15000)
12971276
Sleep(5);
12981277
if (pending->done.load(std::memory_order_acquire))
@@ -1483,8 +1462,7 @@ namespace {
14831462
it.quantity = item->quantity;
14841463
it.equipped = item->equipped;
14851464
it.item_id = item->item_id;
1486-
// Keep the dyes so Draw can fetch the correctly-tinted icon on demand; don't decode
1487-
// the image here (see ApplyItemJson) - it would decode icons for items never shown.
1465+
// stash dyes so Draw fetches the tinted icon lazily (see ApplyItemJson); no decode here
14881466
it.dyes = static_cast<uint32_t>(item->dye.dye1) | (static_cast<uint32_t>(item->dye.dye2) << 8)
14891467
| (static_cast<uint32_t>(item->dye.dye3) << 16) | (static_cast<uint32_t>(item->dye.dye4) << 24);
14901468
it.description.reset(GetItemEncDescription(item).c_str()); // EncString decodes for display
@@ -2274,9 +2252,8 @@ void AccountInventoryWindow::Draw(IDirect3DDevice9*)
22742252
}
22752253
else {
22762254
const auto pos = ImGui::GetCursorPos();
2277-
Item* it = i_front->item;
2278-
// Fetch (and cache) the icon only now that we're actually drawing it. The returned
2279-
// pointer is stable, so this runs once per item - not once per stored item on load.
2255+
const auto it = i_front->item;
2256+
// fetch+cache the icon only when drawn; stable pointer, so once per shown item
22802257
if (!it->texture) {
22812258
const auto player = GW::Agents::GetControlledCharacter();
22822259
it->texture = Resources::GetItemImage(it->model_file_id, it->interaction, it->dyes, player && player->GetIsFemale());

0 commit comments

Comments
 (0)