Log in

View Full Version : 5e Character Wizard is Not Guarding its Functionality



Saagael
October 9th, 2021, 19:49
The D&D 5e character wizard is throwing errors when there's an extension loaded that modifies DataCommon.abilities and DataCommon.ability_ltos. Specifically I've hit errors on lines 151, and 1050 of charwizard.lua, as well as line 12 of charwizard_abilities.lua, but there might be more in there.

The issue on line 151 is that the block below iterates on DataCommon.ability_ltos (which has extra entries from the extension), and then inside the loop it tries to index summary.subwindow by the value from DataCommon.ability_ltos. Summary.subwindow does not have extra entries in it for the new abilities (because why would it, we don't care about the new stats for character building), so it throws an 'attempt to index field (a nil value)' errors.


for k,v in pairs(DataCommon.ability_ltos) do
if summary.subwindow then
summary.subwindow["summary_race_" .. string.lower(v)].setValue(CampaignRegistry.charwizard.race.abiliti es[v]);
end
end

One possible solution for this issue would be to add an extra check on the if statement, like such:

if summary.subwindow and summary.subwindow["summary_race_" .. string.lower(v)] then

The above error also appears on line 1050 of charwizard.lua, and the onInit function of charwizard_abilities.lua. There are attempt to index subwindows by abilities in the abilities arrays. I was able to fix the errors by adding if statements checking to see if the subwindow exists (as in the example code above).

Zacchaeus
October 9th, 2021, 19:59
It's the responsibility of extension authors to write their code to suit the ruleset - not the other way around. You should report this to the extension author.

Saagael
October 9th, 2021, 20:03
It's the responsibility of extension authors to write their code to suit the ruleset - not the other way around. You should report this to the extension author.

In general I agree, but to solve this issue from the extension would be to either overwrite entire functions (one of which is the commitCharacter function and is over 400 lines long), or to rename DataCommon.abilities and DataCommon.abilitiy_ltos, both of which are used all over the ruleset. Those both seem like quite dangerous changes for an extension to make.

MeAndUnique
October 10th, 2021, 17:48
From my perspective this is primarily a situation of an opportunity for SW developers to consider feedback from a community developer interacting with a system that is in development. The code for the character wizard is quite cumbersome to interact with as an extension developer. So while I can agree that the onus to make specialized functionality lies with the extension developer, certainly, I also posit that this particular situation represents an opportunity for the code quality of the ruleset to be examined and potentially improved, which would benefit all parties involved in the long run.

Given that the 5E ruleset has a number of disparate locations when it is using relatively simple data structures for managing ability score information, one potential solution would be to add a new manager for abilities. This would both allow the ruleset to be more declarative (which benefits all developers internal and external) and provide a dedicated space for extension developers to consider their specializations.

As a semi-related aside, I have a couple of friends who work as developers in their day job who have opted not to buy in to FG solely because the development environment is not satisfying to work in when compared against modern frameworks and code-styles. I'm sure this represents a very small minority of the customer base though.