Skip to content

Refactor stringify, add tests for it, and use it in some exports#2347

Merged
LocalIdentity merged 8 commits into
PathOfBuildingCommunity:devfrom
vaisest:export-refactor
Jul 11, 2026
Merged

Refactor stringify, add tests for it, and use it in some exports#2347
LocalIdentity merged 8 commits into
PathOfBuildingCommunity:devfrom
vaisest:export-refactor

Conversation

@vaisest

@vaisest vaisest commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Description of the problem being solved:

Doesn't solve anything, but I think this makes it easier to export files, which this program does a lot of of. I think the style of manually writing output files character by character is pretty difficult to work with, and something like this means you can just build a table and do utils.saveTableToFile(path, table, description) . See flavourText.lua for an example. This makes it MUCH faster to add more data imports later. For example adding a field which contains a table later is very easy

This also rewrites stringify() and makes it work with a lot more tables. For example multi-line string keys, table keys, mixed tables work now. Some of the table types that Lua supports like function keys were skipped. There is added whitespace, which formats files in about the same way as JS's stringify(obj, null, "\t") would:

{
	["HandWrapsCriticalMultiplier4"] = {
		"+(2.1-2.5)% to Critical Hit Chance",
		["affix"] = "of Fury",
		["group"] = "BaseCriticalHitChance",
		["level"] = 1,
		["modTags"] = {
			"critical",
		},
		["statOrder"] = {
			1355,
		},
		["tradeHashes"] = {
			[1909401378] = {
				["canonicalStat"] = "additional_base_critical_strike_chance",
				["statValues"] = {
					["additional_base_critical_strike_chance"] = {
						["max"] = 250,
						["min"] = 210,
					},
				},
			},
		},
		["type"] = "Suffix",
		["weightKey"] = {
			"default",
		},
		["weightVal"] = {
			0,
		},
	},
}

While this looks crazy on the loc meter, I think this is also better because diffs will highlight fewer differences, and this will probably help with conflicts when e.g. rebasing a branch.

Steps taken to verify a working solution:

  • Old tests pass
  • New tests were added

Link to a build that showcases this PR:

Before screenshot:

After screenshot:

image

@vaisest
vaisest force-pushed the export-refactor branch from 262f00e to 3cde972 Compare July 4, 2026 11:34
@vaisest

vaisest commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

3cde972 converted mods.lua to use this. This was mostly done to test performance (it's now only a bit slower, probably because the string is saved in memory instead of being written directly). If it's not ok, feel free to revert the commit.

It does change the style of the written file as originally each mod was on it's own line. I think this might be an improvement since the statOrder seems to often change and it makes finding new/removed mods from github diffs pretty hard.

I've not replaced writeLuaTable() here even though it fills a similar function. It seemed to have some custom export logic so I left it alone

vaisest and others added 3 commits July 7, 2026 19:16
This reverts commit c0b3cf1.
Use Lua's quoted-string formatting to safely escape quotes, backslashes,
control characters, and newlines. Remove the now-redundant manual escaping from the flavour text exporter and add tests for strings and keys.
Fixes the export of tags for some mods as I didn't realise that was an issue
@LocalIdentity LocalIdentity added technical Hidden from release notes pob1 Should also be merged into PoB1 Repo labels Jul 11, 2026

@LocalIdentity LocalIdentity left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed a minor issue with some lua string stuff and also removed the mod export changes
It did highlight that we were missing the export of mod tags for some mods though so I fixed that

@LocalIdentity
LocalIdentity merged commit 9065278 into PathOfBuildingCommunity:dev Jul 11, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pob1 Should also be merged into PoB1 Repo technical Hidden from release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants