infinitesunrise
commented
11 months ago Ah yea I see, Varpuluuta's CellEaterComponent lives in a child entity, but NT's serialization function assumes that all child entities on a wand are spells. So the action_id for the cell eater child entity serializes a nil since it doesn't have an ActionComponent. Agreed this is fixable, a check for action_id ~= nil and item_action_component ~= nil before serializing a spell, at minimum!
In vanilla Noita, there are few enough of these special cases that I think they can be handled by GetWandSpells() on a case-by-case basis as a third return variable (local always_cast, deck, special_case = GetWandSpells(wand) and special_case can be added to the json only if not nil, with a complimentary unpack check for this variable in SpawnWand() on the receiving end)
Regarding the Wand Experimenter exploit: Rather than serializing stat_times_player_has_edited and stat_times_player_has_shot, I favor setting these values to 1 and 10 respectively for all wands withdrawn from bank as I don't think they should ever be eligible for use with that perk (Plus most of those wands are unused anyway, so serializing the extra values is ultimately a waste of resources).
As per modded content on wands, here's a basic strategy: 1) Serialize any action_id you find. 2) If a game client encounters an action_id during unserialization that it doesn't recognize, just skip adding it to the wand during spawn (Not our fault if y'all aren't running the same mods!) 3) Support a tag, say "ntmodspell" or something, where if that tag is added to a wand's child entity then GetWantSpells() will YOLO and serialize the whole damn thing as an xml string instead of just recording an action_id. Maybe up to a certain character limit per spell, for sanity.
Turns out this wand ("Varpuluuta", one of the rare vault puzzle rewards) breaks NT in a couple of ways :sweat_smile: simply holding it kills the mod's local websocket async coroutine when it tries to serialize the player's wands. Even if you can then get it into the bank (throw it in the air and mod restart before it lands on the deposit), it serializes the special passive stain-removing effect as a bogus spell. That's fixable, but at the mod level I can't stop the stain-cleaning effect from being lost when banked.
Which kind of ties back to my first experiment from a random thought (and TBH I'm sure someone has already figured this out): wand experimenter is wildly overpowered in NT because banked wands do not save their AbilityComponent
stat_times_player_has_editedorstat_times_player_has_shotvalues. which means if you have wand experimenter, every bank-withdrawn wand is considered unedited and never cast, which means (through re-banking) unlimited healing from even one wand :sweat_smile:Which then ties back to, in a sense, the way items, spells and wands are stored currently is somewhat flawed / limited. It's just a simple representation that only handles the basics (stats, spells, charges). Anything more complicated (unexpected child entities like with Varpuluuta, unexpectedly modified component values) at best doesn't work. Modded content is also limited, something that would be good to support if possible.
On the other hand, supporting arbitrary components and child entities has its own set of issues. Foremost is size: the XML (as generated by Component Explorer) of a random Varpuluuta I spawned is about 66Kbytes, and includes a lot of probably irrelevant / redundant information (all spark bolts are(?) the same, for instance). LuaComponent could also theoretically be dangerous, though I'm not sure how much so. LuaComponent doesn't accept arbitrary Lua, just a preexisting script path; if you're installing a mod with malicious Lua scripts, it's probably not going to wait for a weird custom item to be put in the NT bank to fire off...
Anyway, not really one real issue (besides Varpuluuta breaking the mod and wand experimenter being overpowered), but more of a greater thought on the state of the bank in NT as it stands.