This might be a dumb question if you've been around for a while. Sorry if that's the case.

I figured I'd take a stab at loading the spell book from classic saves since it's basically just stubbed out with a TODO right now. I understand someone else may be working on this but I just wanted to try something.

When I gather up all of the SpellRecord entries from the SaveTree.FindRecords(RecordTypes.Spell, null) I see a set of records that's a super-set of my spellbook in my classic save file. There are some that I don't recognize at all and some that are clearly the custom names I gave the Spell Maker when I made the spells.

The strange thing is, the length of the SpellRecord.spellName field is FIXED at 25 in the case where the spells came from my spellbook and VARIABLE length when they aren't. The one's that are FIXED length are 'null' padded but the nulls are embedded in the C# string.

A couple (perhaps dumb) questions:
- Is there a utility somewhere for converting these fixed length string records into valid C# strings?
- Is there some way in Visual Studio to SEE this in the debugger? I'm new to C# (first line of code written today )and it took me a while to figure out what was going on with these strings. If this were Java in Eclipse I would have SEEN the nulls in the strings in the debugger since it expands the strings into an array characters.

This is a common enough situation that my FileProxy class has a helper called ReadCString() to handle it. It will read the full length so reader position is calculated correctly, then do the null termination so the string output is also correct.

If you're reading file via the usual API interfaces, you should have a reference to FileProxy somewhere to call this helper from.

The file is already parsed and sitting in records in StartGameBehaviour.StartFromClassicSave() as part of the saveGames local variable (saveGames is a the property SaveGames.SaveTree and seems to be loaded from the SaveGames.TryOpenSave(classicSaveIndex) method). I'm retrieving the list by type using:

Each record in the return is actually the type SpellRecord so I'm assuming the task amounts to mapping from these SpellRecords to an array of EffectBundleSettings and passing these to playerEntity.DeserializeSpellbook at the appropriate place.

I'll see what I can do. I'm curious what these non-spellbook spells are and how to tell the difference from the records. It's not obvious. I might need to dig into where they're parsed anyway if the two spell types are from different sections of the save file. In that case I can add a field to distinguish.

I'll give it a shot. Let me know if you think this is a bad approach. Also, do you know if anyone else is working on it? If not I'll try to do it clean enough for a PR.

Allofich is the one most involved with reading classic save data these days. I'm not sure if he's planning to implement this, but I can't recall him ever mentioning it.

It's been on my backburner until I completed all the classic spell effects. This is something I would likely have started looking into next month. I'm hoping to finalise classic spell effects (excepting Lycanthropy) by coming weekend. I just need work to stop kicking my butt for a few days so I have the head-space left over to get some coding done at night.

I'm at work right now and can't look deeply at the code. From memory, the player's spellbook records are held under the spellbook item itself (which is a container record with the spell records as children). You can use the "save explorer" tool in DFTFU to view the save hierarchy to confirm.

The approach should be to locate spellbook item container record, read in the spell record children that, then convert them to bundle settings and add to DFU spellbook.

You were already using your 'ReadCString' to read the spell name. The problem appears to be the assumption that a C-String is null padded rather than just null terminated. It appears that AFTER the null termination, anything that happened to have been in memory could end up in the record. Either that or there was a memory overrun or missalignment in Daggerfall that caused the last characters to be non null.

You were right about the spell record data not being fully null padded. Good catch! The most appropriate method for this one is ReadCStringSkip() which can read up to null terminator then skip over remaining length regardless of padding. I had to make this one static, which is possibly why it wasn't used originally.

As for ReadCString(), it should be used when reading a fixed non-null-terminated string or a buffer fully padded with nulls. It wasn't the best choice for reading the spell name in this case. You were also right about the redundant trim in DaggerfallSpellReader.ReadSpellData(), another good catch.

While digging into this, I kind-of implemented classic spell import as part of the process. I'm sorry to step on your progress here. Just the only way I could be sure I was looking into this properly was to repeat part of the work, and it was only a few dozen lines of code to setup classic spell import with the helpers already in place. It all just fell into place without really intending it to do that from the outset.

No problem. I was just looking to dig into the code somewhere. 100% of my Unity and C# experience is now in playing with this which was actually the point. The other place I was going to tackle was trying to place the player on the right place when loading (my character ends up under structures if they were on top of the structure when the game was saved).. But that seemed like a much bigger first task.

The only other thing was I implemented a lambda based search on the SaveTree so, for example, finding the spellbook parent Item was just a 1-liner:

This would collapse all of the FindRecord* methods to simple convenience methods with specific lambdas.

And one last thing. It seems ReadCStringSkip() (I didn't look at the code so I'm assuming it just reads to the null or end and advances the stream using Skip the rest of the way) is a less error-prone superset of ReadCString. Why not collapse them and always do the skip? There isn't a case that I can see where assuming all of the bytes past the first null are also nulls matters and it's likely to bite you if the assumption is wrong (case in point ).