Changing Database Schemas on published rulesets - discussion

[phantomwhale]

As Savage Worlds ruleset grows, earlier database schema decisions have become "bumps" in the road for nice, reusable widgets.

The strongest case of this is NPC and PC database structures, which with newer functions to copy NPCs to PCs, and vica versa, means some translation each time. This also leads to a lot of "if type == npc then get the abilityname node, else get the name node" coding where the target could be an NPC or PC, such as on a combat tracker.

So my hope for the next version of Savage Worlds ruleset is I could find some way of progressing the database schema, so I can reduce the conditional logic in my code, without breaking backwards compatibility.

What I was therefore hoping to get from this thread is; Has anyone already tried to tackle this ? Are there any good ideas for doing this ? Is it even worth doing ?

My first attempt at making NPC "abilities" the same as PC "edges" and "hindrances" is to change the "abilityname" and "abilitydescription" nodes to just "name" and "description". I did this in the NPC main sheet as follows:



<sheetdata>
<!-- Maintain old ability name elements for backward compatibility -->
<hs name="abilityname">
<script>
function onInit()
if getValue() ~= "" and window.name.getValue() == "" then
window.name.setValue(getValue())
end
window.name.getDatabaseNode().onUpdate = update
end

function update()
setValue(window.name.getValue())
end
</script>
</hs>
<hs name="abilitydescription">
<script>
function onInit()
if getValue() ~= "" and window.description.getValue() == "" then
window.description.setValue(getValue())
end
window.description.getDatabaseNode().onUpdate = update
end

function update()
setValue(window.description.getValue())
end
</script>
</hs>
<stringfield name="name">
<anchored>
...
</anchored>
<font>sheettext</font>
<script>
...
</script>
</stringfield>
<stringfield name="description">
<anchored>
...
</anchored>
<font>sheettextsmall</font>
<script>
...
</script>
</stringfield>


This worked fine up to a point - today I realised this WON'T work on library modules where the NPC has been provided as a static (read-only) entry. In that situation, the window.name.setValue() function will silently do nothing, leaving an NPC with a bunch of blank special abilities.

So currently it seems I'd need to check EVERY Savage World published product for static NPC entries, and ensuring they are updated in lock-step with the new Savage Worlds version. I'm not sure what other approach would work here - and it might leave some fan-created library modules not working. (To be clear, exported campaign modules should be fine, as they are never static).

Perhaps I need to just embrace the legacy structure and live with it - but it does make some of the code less readable and reusable, so it's an issue I'd like to discuss before giving up on it.

[Ikael]

I don't understand why static library modules leave just blank abilities? I have been struggling very much with static database nodes when I was developing SW Enhanced Library features, but they are managable. Maybe you should think on the other side. Don't attempt to use "non-existing nodes in library", instead "setup non-existing nodes basing on library entry nodes". If you could provide more detailed example about the issue I think I could help you. Alternatively you can check SW Enchanced Library extensions for some freaking amount of library-based changes :D I must say that's still not commercial product releaseable, but it works (thou in v3.4 I have been some small errors, but they don't matter in this case).

[phantomwhale]

The reason it fails (with the approach I took) was:

(a) the NPC window class shows the "name" database node value - this is typically not set on older code, as the database node used to be called "abilityname"
(b) to get around this, whenever I'm displaying an NPC's abilities, I check the ability to see if it has a blank / empty value for the "name" database node. If so, then I copy in the value from the "abilityname" node, such that they match.
(c) Therefore, the node should display with the correct text, as the "name" database node is now populated

However, with a static library node, all database updates are silently ignored (as the data is read-only). So step (b) fails to copy the value from the "abilityname" node, meaning the "name" node remains blank, and nothing is displayed.

[Doswelk]

May I make an "out-of-the-box" suggestion...

Why not break backward compatibility if it makes things easier in the future and more modular?

AS LONG as some tool (extension) can be provided to convert old NPCs to the new format?

It is not ideal but what are your thoughts?

[phantomwhale]

Ok, thinking about a non-backward compatible change, I need to address the following sources that will conform to the old database schema:

(1) Existing SW campaigns
(2) Homemade SW exported modules
(3) Published SW exported modules
(4) Homemade SW libraries
(5) Published SW libraries
(6) Homemade SW extensions
(7) Publish SW extensions

These can be handled as follows

(1) I've written a migrate manager lua script, which detects pre-v3.4 campaigns and automagically fixes them as they are loaded - so thats DONE
(2/3) These should automatically repair themselves when they are loaded into the campaign (by the script copying the abilityname node into the name node)
(4/5) These will automatically repair themselves as above, UNLESS the data is held in a static node, in which case work is needed
(6/7) These will need to ensure they also include the code in the NPC window class to read from both the old and new node name (as posted in my initial post)

So after releasing SW v3.4, updates could be applied to existing libraries to make them meet the new format going forward. But as you say, having a tool to automate this would be ideal !
That means republishing every library / extension and module in the store though - although not all on day 1. This fixes points (3,5,7) permanently.
And for homegrown modules / libraries, we can provide a tool / guide to upgrade.

[Ikael]

Here is an alternative solutions for review purposes. I found out that you can avoid static annoyance by playing directly with databasenodes. I created very simple scripts migration.lua which handles the migration work. The script also defines set of tag rules which can be used to define what nodes are going to be migrated. See the attachment. It contains the script and sample npc_main.xml from SW v3.4 which uses the script in several places:

In your original place where abilityname and abilitydescription is migrated into name and description
In combat page where all attributes are being migrated to be the same named as in charsheet
In skilllist where skillname is migrated to be name, for sake consistency of naming between charsheet and npc sheet


I tested this with campaign material, exported campaign material, static module materials and it seemed to work, but I cannot be 100% sure. Reviewing would be nice.

PS: Phantomwhale I made pull request for you to easy reviewing

[Ikael]

migration.lua script's content



function onInit()
if migrations and migrations[1] then
for _, migrationdatas in pairs(migrations[1]) do
local source = nil
local target = nil
for _,data in pairs(migrationdatas) do
for name,values in pairs(data) do
if name == "source" then
source = values[1]
elseif name == "target" then
target = values[1]
end
end
if source and target and getDatabaseNode() then
local targetnode = getDatabaseNode().getChild(target)
if targetnode and targetnode.getType() then
if targetnode.getType() == "number" then
migrateNumberNode(source, target)
elseif targetnode.getType() == "string" then
migrateStringNode(source, target)
elseif targetnode.getType() == "dice" then
migrateDiceNode(source, target)
end
end
end
end
end
end
end
function migrateNumberNode(oldnodename, newnodename, defaultvalue)
if not defaultvalue then
defaultvalue = 0
end
migrateNode("number", oldnodename, newnodename, defaultvalue)
end
function migrateStringNode(oldnodename, newnodename, defaultvalue)
if not defaultvalue then
defaultvalue = ""
end
migrateNode("string", oldnodename, newnodename, defaultvalue)
end
function migrateDiceNode(oldnodename, newnodename, defaultvalue)
if not defaultvalue then
defaultvalue = {"d4"}
end
migrateNode("dice", oldnodename, newnodename, defaultvalue)
end
function migrateNode(nodetype, oldnodename, newnodename, defaultvalue)
if not getDatabaseNode() then
return
end
local oldvalue = DB.getValue(getDatabaseNode(), oldnodename, defaultvalue)
local newvalue = DB.getValue(getDatabaseNode(), newnodename, defaultvalue)
if nodetype == "dice" then
oldvalue = StringManager.convertDiceToString(oldvalue)
newvalue = StringManager.convertDiceToString(newvalue)
defaultvalue = StringManager.convertDiceToString(defaultvalue)
end
local newnode = getDatabaseNode().createChild(newnodename, nodetype)
if newnode and oldvalue ~= defaultvalue and newvalue == defaultvalue then
print("Migrating '" .. oldnodename .. "' to be '" .. newnodename .. "' with value '" .. tostring(oldvalue) .. "'")
if nodetype == "dice" then
oldvalue = StringManager.convertStringToDice(oldvalue)
end
newnode.setValue(oldvalue)
end
local oldnode = getDatabaseNode().getChild(oldnodename)
if oldnode then
print("Migrated old node " .. oldnode.getNodeName() .. " deleted")
oldnode.delete()
end
end

[Ikael]

Samples how to use it in xml code



<windowclass name="npc_abilityentry">
<sizelimits>
<minimum>
<height>20</height>
</minimum>
</sizelimits>
<migrations>
<migration>
<source>abilityname</source>
<target>name</target>
</migration>
<migration>
<source>abilitydescription</source>
<target>description</target>
</migration>
</migrations>
<script file="scripts/migration.lua"/>
<sheetdata>
<stringfield name="name">
<anchored>
<top/>
<left><offset>5</offset></left>
<right><offset>-25</offset></right>
</anchored>
<font>sheettext</font>
<frame>
<name>textline</name>
<offset>2,0,2,0</offset>
</frame>
<script>
function onGainFocus()
window.description.checkVisibility(true)
end

function onLoseFocus()
window.description.checkVisibility()
end
</script>
</stringfield>
<stringfield name="description">
<anchored>
<to>name</to>
<position>below</position>
<left>
<offset>25</offset>
</left>
</anchored>
<frame>
<name>textlinesmall</name>
<offset>2,-1,2,1</offset>
</frame>
<font>sheettextsmall</font>
<multilinespacing>14</multilinespacing>
<script>
function checkVisibility(force)
if not force and not hasFocus() and getValue() == "" then
setVisible(false)
else
setVisible(true)
end
end

function onGainFocus()
checkVisibility(true)
end

function onLoseFocus()
if getValue() == "" then
setVisible(false)
end
end

function onInit()
checkVisibility()
end
</script>
</stringfield>
....




<windowclass name="npc_combat">
<migrations>
<migration>
<source>agility</source>
<target>Agility</target>
</migration>
<migration>
<source>agilityMod</source>
<target>AgilityMod</target>
</migration>
<migration>
<source>smarts</source>
<target>Smarts</target>
</migration>
<migration>
<source>smartsMod</source>
<target>SmartsMod</target>
</migration>
<migration>
<source>spirit</source>
<target>Spirit</target>
</migration>
<migration>
<source>spiritMod</source>
<target>SpiritMod</target>
</migration>
<migration>
<source>strength</source>
<target>Strength</target>
</migration>
<migration>
<source>strengthMod</source>
<target>StrengthMod</target>
</migration>
<migration>
<source>vigor</source>
<target>Vigor</target>
</migration>
<migration>
<source>vigorMod</source>
<target>VigorMod</target>
</migration>
</migrations>
<script file="scripts/migration.lua"/>
<sheetdata>
<stringcontrol name="abilitieslabel">
<font>sheetlabelsmallbold</font>
<anchored>
<left>
<anchor>left</anchor>
<offset>5</offset>
</left>
<top>
<anchor>top</anchor>
<offset>15</offset>
</top>
<size>
<width>50</width>
<height>25</height>
</size>
</anchored>
<static>Abilities</static>
</stringcontrol>

<npcdiescore name="Agility">
<anchored>
<to>abilitieslabel</to>
<position>right</position>
<offset>10</offset>
</anchored>
<description>Agility</description>
<tabtarget next="Smarts" prev="" />
</npcdiescore>
<hn name="AgilityMod"/>
<npctoplabel>
<anchor>Agility</anchor>
<static>Agi</static>
</npctoplabel>
....




<windowclass name="npc_skillentry">
<sizelimits>
<minimum>
<height>25</height>
</minimum>
<maximum>
<width>140</width>
</maximum>
</sizelimits>
<migrations>
<migration>
<source>skillname</source>
<target>name</target>
</migration>
</migrations>
<script file="scripts/migration.lua"/>
<sheetdata>
<npcdieskill name="skill">
<bounds>5,0,25,25</bounds>
</npcdieskill>
<hn name="skillmod"/>
<stringfield name="name">
<anchored>
<left>
<parent>skill</parent>
<anchor>right</anchor>
<offset>5</offset>
</left>
<top>
<anchor>top</anchor>
<offset>3</offset>
</top>
<right>
<anchor>right</anchor>
</right>
</anchored>
<font>sheettextsmall</font>
<frame>
<name>sheettextlinesmall</name>
<offset>0,-1,0,0</offset>
</frame>
<multilinespacing>18</multilinespacing>
<empty>&#171; Unnamed &#187;</empty>
</stringfield>
</sheetdata>
</windowclass>

[phantomwhale]

I found out that you can avoid static annoyance by playing directly with databasenodes.

Ah ha - here was the thing I was missing !

I can change window.name.setValue(getValue()) to window.name.getDatabaseNode().setValue(getValue()) and static library nodes are no longer a problem.

I'll review the migrations change - I've already written a script that only fires once (rather than everytime you load a window), but I'm worried that by only firing once, you might miss out on extensions data (e.g. Deadland's Critters) if the extension isn't loaded when the updated ruleset is loaded for that campaign.

[S Ferguson]

Ah ha - here was the thing I was missing !

I can change window.name.setValue(getValue()) to window.name.getDatabaseNode().setValue(getValue()) and static library nodes are no longer a problem.

I'll review the migrations change - I've already written a script that only fires once (rather than everytime you load a window), but I'm worried that by only firing once, you might miss out on extensions data (e.g. Deadland's Critters) if the extension isn't loaded when the updated ruleset is loaded for that campaign.

Wow. That was the shortest, in depth code discussion I've seen on the forum. I've always maintained that it's good to work in groups. Problem solving is easier for everyone.:) (The bold type really caught my interest, too)