Author Topic: Converting OXCE to rapidyaml  (Read 3098 times)

Offline Delian

  • Colonel
  • ****
  • Posts: 470
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #45 on: November 07, 2024, 01:55:29 am »
Error in bin/common/SoldierName/Polynesia.nam
last line:
 - 'Akamu

Not valid YAML: unclosed single-quoted flow scalar

Should I wrap it in double-quotes, e.g. - "'Akamu" to fix the error?
« Last Edit: November 07, 2024, 02:00:40 am by Delian »

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3331
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #46 on: November 07, 2024, 02:07:53 am »
if `'` is "letter" and not quote then, Yes

Offline Delian

  • Colonel
  • ****
  • Posts: 470
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #47 on: November 07, 2024, 05:05:34 pm »
Ok. Some more bugs from before:
Armor::load()
_allowsMoving = node["allowsMoving"].as<bool>(_allowsMoving);
should be
loadBoolNullable(_allowsKneeling, node["allowsKneeling"]);
Because in the yaml it's stored as bool, but in program it's loaded as sint8

I also have a problem:
In a few rare cases, in the yaml I have a node with a value 10.0, which is obviously a float, but deserialization crashes because it wants to deserialize into an int. The question is, is this faulty yaml? Or faulty deserialization?

Offline Delian

  • Colonel
  • ****
  • Posts: 470
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #48 on: November 07, 2024, 05:46:59 pm »
Starting up game (XPZ) in debug mode...

Startup time:
Before: 130s
After: 17s

RAM usage in debug mode:
Before: 1450MB
After: 750MB
Before, if you ran XPZ in debug mode and loaded a large save, game would crash if it reached 1.8GB RAM (out of memory), so couldn't even debug certain large saves...

Some screenshots of a profiling run
Before:


After


I haven't tested game saving and loading yet, which is next on the list.

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3331
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #49 on: November 07, 2024, 09:47:29 pm »
Ok. Some more bugs from before:
Armor::load()
_allowsMoving = node["allowsMoving"].as<bool>(_allowsMoving);
should be
loadBoolNullable(_allowsKneeling, node["allowsKneeling"]);
Because in the yaml it's stored as bool, but in program it's loaded as sint8

I also have a problem:
In a few rare cases, in the yaml I have a node with a value 10.0, which is obviously a float, but deserialization crashes because it wants to deserialize into an int. The question is, is this faulty yaml? Or faulty deserialization?
What property exactly? overall if game save `10.0` we should load it as `float` even if we expect `int`, we need keep backward compatiblity, we fix saving.
I think probaby best would be custom function of it like `loadIntFixed` or some thing like that that check for `float` too.

Offline Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 9061
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #50 on: November 07, 2024, 10:03:40 pm »
Before, if you ran XPZ in debug mode and loaded a large save, game would crash if it reached 1.8GB RAM (out of memory), so couldn't even debug certain large saves...

Yeah, happened to me many times.

Btw. why does it crash at 1.8GB if I have 32GB RAM? Something to do with 32bit? Why not 2.0GB?
Maybe a stupid question, but I'm curious.
« Last Edit: November 07, 2024, 10:10:37 pm by Meridian »

Offline Delian

  • Colonel
  • ****
  • Posts: 470
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #51 on: November 07, 2024, 11:24:46 pm »
What property exactly? overall if game save `10.0` we should load it as `float` even if we expect `int`, we need keep backward compatiblity, we fix saving.
I think probaby best would be custom function of it like `loadIntFixed` or some thing like that that check for `float` too.

There are 8 places where Dio accidentally set Item.damageAlter.FireThreshold to a float value instead of int, that's all. No other problematic places for this sort of error in XPZ.
According to the yaml standard, if an untagged plain scalar matches the regular expression -?(0|[1-9][0-9]*), it should resolve to int. So technically 10.0 cannot be int and it's correct parser behavior that parsing fails there.
As far as backwards compatibility goes, that's true. But this migration already breaks compatibility due to the issues with trailing space and trailing tabs. If the modders will have to fix those, we might as well tell them where they put in floats instead of integers.

Btw. why does it crash at 1.8GB if I have 32GB RAM? Something to do with 32bit? Why not 2.0GB?
Maybe a stupid question, but I'm curious.

I think it's got something to do with shared graphics memory. Some address space is reserved so that the application can address graphics card's memory, but then it cannot use those address space itself.

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3331
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #52 on: November 08, 2024, 12:50:53 am »
There are 8 places where Dio accidentally set Item.damageAlter.FireThreshold to a float value instead of int, that's all. No other problematic places for this sort of error in XPZ.
According to the yaml standard, if an untagged plain scalar matches the regular expression -?(0|[1-9][0-9]*), it should resolve to int. So technically 10.0 cannot be int and it's correct parser behavior that parsing fails there.
As far as backwards compatibility goes, that's true. But this migration already breaks compatibility due to the issues with trailing space and trailing tabs. If the modders will have to fix those, we might as well tell them where they put in floats instead of integers.
right, I forget you work with `Mod` and not with `SaveGame`, in this case this was wrong and mod should be updated as it was never expected to accept floats.
Optionally we could add "grace period" of year before we remove loading of `float`s (`Mod` class have special functions for this)

Offline Stone Lake

  • Colonel
  • ****
  • Posts: 179
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #53 on: November 08, 2024, 08:47:48 am »
Quote
Btw. why does it crash at 1.8GB if I have 32GB RAM? Something to do with 32bit? Why not 2.0GB?
Maybe a stupid question, but I'm curious.
32 bit apps in Windows can only allocate 2 GB of memory (4 GB for 32 bit address, 2 GB system space and 2 GB user-space). 2 GB is already past limit, so it crashes a bit before that.
Memory can be increased to 3 GB by using /LARGEADDRESSAWARE linker flag. In fact, I can only launch XPZ in 32-bit OXCE with it enabled.

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3331
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #54 on: November 08, 2024, 11:13:51 am »
ok, I recalled that 64bit had split of address space where "negative" values belong to OS and rest of to user space.
Add reserved memory by hardware and we end up with 1.8GiB possible memory to use on 32bit OS per process.

I think we could anyway add some thing like this:
https://stackoverflow.com/questions/14035065/codeblocks-compile-with-large-adress-aware-flag

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3331
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #55 on: November 08, 2024, 04:31:14 pm »
There are 8 places where Dio accidentally set Item.damageAlter.FireThreshold to a float value instead of int, that's all. No other problematic places for this sort of error in XPZ.
According to the yaml standard, if an untagged plain scalar matches the regular expression -?(0|[1-9][0-9]*), it should resolve to int. So technically 10.0 cannot be int and it's correct parser behavior that parsing fails there.
As far as backwards compatibility goes, that's true. But this migration already breaks compatibility due to the issues with trailing space and trailing tabs. If the modders will have to fix those, we might as well tell them where they put in floats instead of integers.

I think it's got something to do with shared graphics memory. Some address space is reserved so that the application can address graphics card's memory, but then it cannot use those address space itself.
I make some digging and I see that I make some mistake in code as property is `int` but loaded as `float`, this mean Dio is technically correct using `10.0`.
Now I only need `int` as final damage is always integer but on other hand is easy make mistake as only two of dozen of properties there are `float` there.

Offline Delian

  • Colonel
  • ****
  • Posts: 470
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #56 on: November 09, 2024, 12:08:02 am »
Hmm... oh, maybe that's why yaml-cpp didn't throw an error before.

Anyway, man, debugging yankes code is so hard because all the variable names are a single letter. Also, I'm basically done. Loaded+saved a few large battlescape sav files, and the output was basically identical. I can pull the code to my git fork now if you want.

Everything works so nice and fast now... well, almost everything. Script parsing and vector sorting are now the major factors of game startup. Sorting wouldn't be necessary if everyone just used the correct data structures in the first place, but that's a problem for another time.
Btw, Yankes, in Script.cpp:694 - parse(), you have a stringstream there. That ss alone now consumes 4% of the whole game load time lol

Now where was I... ah, I gotta download and test all the other mods...

Offline Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 9061
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #57 on: November 09, 2024, 12:16:23 am »
Yes, please push the code to your remote repo for review.

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3331
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #58 on: November 09, 2024, 01:10:26 am »
Hmm... oh, maybe that's why yaml-cpp didn't throw an error before.

Anyway, man, debugging yankes code is so hard because all the variable names are a single letter. Also, I'm basically done. Loaded+saved a few large battlescape sav files, and the output was basically identical. I can pull the code to my git fork now if you want.

Everything works so nice and fast now... well, almost everything. Script parsing and vector sorting are now the major factors of game startup. Sorting wouldn't be necessary if everyone just used the correct data structures in the first place, but that's a problem for another time.
Btw, Yankes, in Script.cpp:694 - parse(), you have a stringstream there. That ss alone now consumes 4% of the whole game load time lol

Now where was I... ah, I gotta download and test all the other mods...

it was 0.4% before :P
Beside vector sorting should be in most cases `O(n*log(n))` plus access `O(log(n))` and its effective "flat-map"  (in case comparing only pointers adresses).
I do not see big potential (max `log(n)`) in speedup as current version is very cache friendly. Of core this is only my assumption and could not match reality fully but most C++ talks say that vector is best data structure even if you remove elements some times.

I think I could drop this last 4% to by switching to some better int parser in scripts.

Offline Whispers

  • Sergeant
  • **
  • Posts: 43
    • View Profile
Re: Converting OXCE to rapidyaml
« Reply #59 on: November 09, 2024, 02:29:11 am »
Hmmm... maybe worth knowing, but maybe not... I've switched a lot of the resource loading and unloading to Json for the Lua/Vulkan branch. Granted, it's not a released branch and I'm still a fair ways away from releasing it, but I thought it might be worth mentioning.

The way I deal with save game, mod loading(rulesets and so on) and config stuff is through serializers which are either hand crafted or auto-generated from the run time type information. What this means is that there is a considerable increase in compile times and executable size due in large part to the compilation and linking of a code generator which then needs further compilation, but saves time on loading during runtime. For Lua and Json, all the bindings are automatic, but since a goal of the Lua/Vulkan project is to retain backwards compatibility, then I manually put in the YAML serializers so they conform to the original OXCE rules. In fact, it makes it nicer because I can put whatever I want in the Json and not have to worry if I might break some mod out there that someone wrote a long time ago. Granted, I've only made it as far as the config stuff at the moment(config, localization, and a few other bits), so I might run into some issue down the line which might make it more of a problem, but working with Json in debug is so much faster than YAML. However, equally, it is worth mentioning that I barely notice YAML in release mode.

Use this information however you see fit. I'd be happy to see SOMETHING improve debug loading and unloading of mods. The Lua/Vulkan branch is really broken at the moment and I won't make any promises as to when it will be released, so there's also that to consider :D