Quick-loot (Union-style): implementation notes and questions before PR

[zxcchel]

Quick-Loot for OpenGothic — summary of changes

An optional "vacuum" looting mode: hold RMB to pick up items from the ground without the bend-down animation, or to empty a container in one press. Picked-up items are shown as a stacked list in the bottom-right corner.

Inspired by the Quick-Loot feature of Union (Gothic 2 mod platform).

---

Behaviour

Action	Result
RMB on an item	Item goes to inventory instantly, no ItmGet animation, works while walking/running
RMB held down	Consecutive items are picked up (150 ms cooldown between pickups)
RMB on a container	All contents transferred at once
RMB on a corpse / unconscious NPC	All lootable contents transferred at once
RMB on a locked container	Refused, shows Locked
RMB while an enemy is actively attacking the player	Refused, shows Not now!
LMB	Unchanged vanilla behaviour (animated pickup, container opens normally)

Message list merges identical items (Blue bloodwort x3), keeps up to 5 lines, and resets after 2.5 s of inactivity.

---

Files changed

1. game/game/playercontrol.h / playercontrol.cpp — the feature itself

New methods:

• bool quickLoot(Item& item) — instant pickup of a single item.
• bool quickLootContainer(Interactive& mob) — transfers the whole container inventory.
• bool quickLootNpc(Npc& other) — transfers the whole inventory of a downed NPC, iterating with Inventory::T_Ransack (the same iterator type the ransack UI uses). Reuses the existing Npc::addItem(size_t, Npc&, size_t) wrapper.
• void pushLootMessage(size_t clsId, const std::string& name, size_t count) — builds and renders the accumulating pickup list.

tickFocus() — a new branch before the existing ActionGeneric (LMB) logic:

if(ctrl[Action::Parade]) {
  static uint64_t lastLootTime = 0;
  auto     w    = Gothic::inst().world();
  uint64_t now  = (w!=nullptr ? w->tickCount() : 0);
  if(w!=nullptr && (now<lastLootTime || now-lastLootTime>150)) {
    if(currentFocus.item!=nullptr && quickLoot(*currentFocus.item)) {
      lastLootTime = now;
      clearFocus();
      return;
      }
    if(currentFocus.interactive!=nullptr && currentFocus.interactive->isContainer() &&
       quickLootContainer(*currentFocus.interactive)) {
      lastLootTime = now;
      clearFocus();
      return;
      }
    if(currentFocus.npc!=nullptr && currentFocus.npc->isDown() &&
       quickLootNpc(*currentFocus.npc)) {
      lastLootTime = now;
      clearFocus();
      return;
      }
    }
  }

Notes on the details, each of which was an actual bug found during testing:

• Action::Parade is used because in gothic2.ini RMB maps to keyParade (keyParade=cf000d02, 0x0d02 = ButtonRight). keyActionRight is d100, a code that is not present in KeyCodec::keys at all, so Action::ActionRight is currently unreachable. This is the weakest part of the patch — see "Open questions" below.
• No clearInput() — it calls movement.reset(), which stops the character mid-run. Only the loot action itself is throttled.
• Cooldown instead of clearing ctrl[Action::Parade] — the flag is only set on key press, so clearing it breaks pickup while the button is held.
• clearFocus() after a successful pickup — the Item is destroyed by takeItem, leaving a dangling currentFocus.item otherwise (caused a crash while running).

interact(Item&) — reverted to plain vanilla behaviour; an earlier local modification that printed a message on LMB pickup was removed.

Localisation — all feature strings go through:

static std::string quickLootText(std::string_view key, std::string_view fallback);

Defaults are English ASCII; they can be overridden in gothic.ini:

[QUICKLOOT]
msgBusy=...
msgLocked=...
msgEmpty=...
msgUnknownItem=...

Item names themselves come from Item::description(), so they are already localised.

2. game/world/objects/npc.h / npc.cpp — animation-free pickup

Npc::takeItem(Item& i) → Npc::takeItem(Item& i, bool noAnim=false). Default argument keeps every existing call site byte-identical.

When noAnim==true:

• setAnimAngGet(Anim::ItmGet, ...) and the trailing implAniWait(...) are skipped;
• the body-state check additionally allows BS_RUN and BS_WALK, so pickup works while moving. The animated path keeps the original BS_STAND / BS_SNEAK / BS_SWIM / BS_DIVE restriction unchanged.

3. game/world/objects/interactive.h / interactive.cpp — lock check

New method:

bool Interactive::isLockedFor(const Npc& pl) const {
  if(!locked || isLockCracked)
    return false;
  const size_t keyInst = keyInstance.empty() ? size_t(-1) : world.script().findSymbolIndex(keyInstance);
  if(keyInst!=size_t(-1) && pl.inventory().itemCount(keyInst)>0)
    return false;
  return true;
  }

The existing needToLockpick() is not suitable here: it returns false when pickLockStr is empty, so a container locked with a key only would be reported as open.

4. game/ui/dialogmenu.cpp — screen message rendering

Two changes, both arguably bug fixes independent of quick-loot:

a) Multi-line support in paintEvent(). PScreen::txt is now split on '\n' and drawn line by line, with the block height taken into account when anchoring to screen edges. Single-line messages are unaffected.

Important detail: the split parts are stored as std::string, not std::string_view. With a non-null-terminated string_view, GthFont::drawText resolves to the const char* overload and prints everything up to the buffer's null terminator, so every line rendered as "itself plus all following lines".

b) printScreen() replaces an entry with identical x/y instead of pushing a new one. Without this, each update of the loot list is drawn on top of the previous, still-alive entries.

This is the change most likely to need discussion: it alters behaviour for any Daedalus PrintScreen call, not just this feature.

5. game/gothic.h — visibility only

Gothic::printscreen(...) moved from private to public. No implementation change; it was previously reachable only from the Daedalus VM bindings.

---

Compatibility

• All new parameters have defaults; existing call sites are untouched.
• LMB interaction, dialogues, parrying and normal container opening were tested and behave as before.
• Save format is not affected.

Open questions

1. Key binding. Reusing Action::Parade works only because canInteract() rejects the case where a weapon is drawn, so it never collides with blocking in practice. A dedicated Action plus its own [KEYS] entry would be cleaner — happy to rework.
2. Feature toggle. Should this be behind a setting and off by default, to preserve vanilla behaviour?
3. DialogMenu::printScreen de-duplication — acceptable as a general change, or should the loot list own its rendering path instead?
4. Movement during pickup. Allowing BS_RUN/BS_WALK is a deliberate deviation from original Gothic mechanics; it is the point of the feature, but it is a behaviour change.
5. Corpse looting bypasses InventoryMenu::ransack(). quickLootNpc() transfers items directly instead of going through the ransack UI path. If that path carries side effects worth preserving (perceptions, theft/witness reactions, script hooks), they are currently skipped — pointers welcome on what should be replicated. Whether T_Ransack is the right iterator type here is also worth confirming.

[Try]

Hi, @zxcchel !

Interesting suggestion, however you don't have to go thru implementation details - that is what code-review is for :)
Is there a particular reason to follow Union's Quick loot, rather one from SystemPack?

Choosing between those tow, SystemPack naturally get the priority.

PS:

// systempack.ini
[PARAMETERS]
...
RMB_No_Take_Anim=0
; ... Forces assignment of LMB and RMB (any other key will not work), disables the selection of animations for the RMB - on (1) or off (0), default is 0.
; WARNING: The engine only supports assigning two buttons per action. These options are mutually exclusive, simultaneously only one of them may operate!
; if both are enabled, only RBM_No_Take_Ani will work, since RBM_No_Take_Ani overrides No_Take_Ani (!)
; if RBM_No_Take_Ani=1 is enabled the action GAME_ACTION is assigned to the left and right mouse button, if assigned to another key (including those on a keyboard) this key will not work in this case (!)
; if you want Ctrl, but do not want the animation, use: No_Take_Anim=1, RMB_No_Take_Anim=0.
; if you do not want the animation bound to RMB, use: No_Take_Anim=0, RMB_No_Take_Anim=1.
; if you want the animation, use: No_Take_Anim=0, RMB_No_Take_Anim=0.

[zxcchel]

Just to make sure I understand correctly — as far as I know SystemPack doesn’t have a quick-loot feature as such; No_Take_Anim / RMB_No_Take_Anim only control the take animation and the LMB/RMB binding, right?
If so, those two options cover part of what I implemented (instant pickup on RMB), but not the rest: a pickup list in the corner, emptying containers and corpses in a single press, and pickup while moving.
So — should I implement exactly the SystemPack options and leave the rest out, or is there interest in the wider feature as an opt-in setting?

[Try]

Exact implementation is desirable. It helps to avoid fragmentation, when one engine is very different to the other.
Wold be nice to first merge systempack-style first - that should be easy to review. And then expand to union-style quick loot.