Norbyte / ositools

Advanced scripting and mod support for Divinity Original Sin 2
MIT License
384 stars 30 forks source link

SyncSkillStat(): Unsupported SkillType: Cone #21

Closed Shresht7 closed 4 years ago

Shresht7 commented 4 years ago

I've been working on a mod that reads a user-created JSON file and overrides the corresponding stat-entries on-the-fly. I encountered an issue when the mod tried to SyncStat() a Cone skill. Got this error message: [Osiris] {E} dse::SkillPrototypeManager::SyncSkillStat(): Unsupported SkillType: Cone

A series of Ctrl-Fs led me here: \ositools\OsiInterface\GameDefinitions\Enumerations.inl

BEGIN_ENUM(SkillType, uint32_t)
    EV(Zone, 0x1)
    EV(SkillHeal, 0x2)
    EV(Jump, 0x3)
    EV(MultiStrike, 0x4)
    EV(Path, 0x5)
    EV(Projectile, 0x6)
    EV(ProjectileStrike, 0x7)
    EV(Quake, 0x8)
    EV(Rain, 0x9)
    EV(Rush, 0xA)
    EV(Shout, 0xB)
    EV(Storm, 0xC)
    EV(Summon, 0xD)
    EV(Target, 0xE)
    EV(Teleportation, 0xF)
    EV(Tornado, 0x10)
    EV(Wall, 0x11)
    EV(Dome, 0x12)
END_ENUM()

There is no SkillType Cone here, could be an oversight. I apologize if I'm completely off the mark.

Norbyte commented 4 years ago

Ah, looks like Cone is actually an alias for Zone, the game uses the same implementation for both.

Norbyte commented 4 years ago

One important comment regarding SyncStat: There are two ways to change stats; the first one is during module load (the ModuleLoading event); when stats are changed during mod load there is no need to sync them as they are not yet in use.

When they are edited after module load you need to explicitly sync them using SyncStat(). One side effect of SyncStat() is that it persists the change in the savegame, i.e. it'll automatically update the changed stat entries from the savegame when you load it, which means that values in the stat .txt are ignored for the skill from that point. I could add a flag to SyncStat to disable savegame writes, though it obviously comes with its own set of caveats.

Norbyte commented 4 years ago

Should be fixed in the latest Devel version.

Shresht7 commented 4 years ago

Yes, I tried both methods of changing stats. Regarding the StatsLoaded/ModuleLoading method: From my testing, this needs to be done in BootstrapClient.lua. But since the mod reads a user-created JSON using Ext.LoadFile() to know what to edit, each client should have the same JSON file locally in Osiris Data for their game to have the same stats. Clients that don't have the JSON or have different content in that JSON, will have different stats from their peers. I tried some PostMessageToClient/Server shenanigans to pass the stringified JSON around to string-compare them, in an attempt to validate the JSON between server-and-clients or simply have the server share their JSON with the clients. But that rabbit-hole led nowhere - as I couldn't figure out how to actually receive the payload string (was able to post and listen though). All my client-side testing has been done by launching a second instance of the game using EoCApp.exe and joining LAN with the first.

Regarding SyncStat baking info into the savegame. I am mostly okay with it. People can override them again to return to the original values, can't they? I was working on a rudimentary history/revert-last-commit feature, but it's on back-burner for now, at least until I sort everything else out first. Also, interestingly, if you activate the "combat-randomizer" Gift-Bag after SyncStat(), it reverts those stat-edits! As for adding a flag to disable savegame-writes, you are best equipped to make that decision.

Since this is done in BootstrapServer.lua, clients don't have to worry about anything. It also allows live rapid-prototyping, I don't even have to save-reload, let alone exit the entire game, to see the changes - it's downright glorious.

An interesting workaround to get the best of both worlds is to create a forked-save, prototype your changes using the SyncStat approach and when you're satisfied, export those changes into a different JSON that would load during StatsLoaded event (you'd, of course, have to share the file with your peers as mentioned before). Then you just delete the forked-save and restart the game. This would be a non-invasive way to use the mod and keep live-editing.

I just fear that all this will overwhelm/confuse the average user. I'm afraid they'll already be put-off by the lack of a UI, simply looking at a JSON or the fact that the mod will do absolutely nothing if they just hit subscribe and never read how to use it.

Thanks for the quick response and fix.

Norbyte commented 4 years ago

The payload should be the second parameter in the network callback:

local function TestHandler(channel, payload)
    Ext.Print("SERVER: " .. channel .. " --> " .. payload)
end

Ext.RegisterNetListener("test", TestHandler)

If you don't want the clients to keep their own copy of the .json files then syncing stats is indeed a better solution. I've added an optional second argument to SyncStat that can toggle savegame persistence when syncing. (i.e. Ext.SyncStat("whatever", false) disables savegame writes).

The gift bag menu essentially triggers a save game -> module reload -> load game cycle, so every stat change is lost, though you can reapply them on SessionLoading. Although it might be a bug as well, since stats reloaded from the save should in theory be automatically synced with clients, which might not happen in the gift bag menu.

Shresht7 commented 4 years ago

Right, thanks. The gift-bag is not an issue for me. Stat import and syncing are done through an Osiris call on demand. Users can reimport the stats whenever. This bug only happened with combat-randomizer (tested with maybe 4-5 different giftbag mods). I can test more If you want me to investigate. I have a few more questions but I'll put them up on discord later, as this issue is resolved. Thanks for everything.