Closed Github-Reneon closed 2 weeks ago
orffen
commented
1 month ago Thank you for the report! I'm quite short on time to be able to work on the Foundry v12 compatibility at the moment, but I do plan on updating the system to work with v12. For the time being please use Foundry v11.
Github-Reneon
commented
1 month ago Hi Orffen. What needs to be done? I can put some time into it if you'd like?
orffen
commented
1 month ago So, what I normally do for a compatibility update is:
1) Create a new branch
1) Load up Foundry v11 (current compatible version) and point to the local git repo's system.json file
2) Open the developer console (F12) and load up the BFRPG world and system
3) Go through all of the warnings/errors and then try to resolve them 🙂
At a guess there are data errors/warnings; the data model changed from v10 to v11 and I think it's also updated in v12. But Foundry is pretty good at throwing warnings about upcoming changes, so resolving those would probably resolve all the compatibility issues.
But, once the warnings/errors are resolved in v11, it's time to switch over to v12 and test. I would also check warnings/errors in the developer console as well and see if errors need to be resolved.
If you do get it all fixed and working, the system.json will need an update with version, compatibility, and your name in Authors 😁 and then you can put in a pull request, and I can merge the changes. I then manually need to update the system on the Foundry website to "publish" the new release.
DC23
commented
3 weeks ago I've had a look at this as well. At verbose logging level, the only error is the new since v12 error about globalThis.mergeObject being deprecated since v12. I haven't looked at the BFRPG system code, but it's easy to imagine that merging objects could be related to persisting the data. The error is happening in BasicFantasyRPGActorSheet.close which looks very suspicious to me.
However, this shouldn't be causing it to fail since it's a deprecation warning for developers. They won't be removing support for the global functions until Foundry v14.
I'm a software engineer, but I've spent my whole career avoiding web and JS development for the most part. I've just spent the last month or two teaching myself Foundry and JS, working from macros to writing a module. I know the v12 API and JS well enough now to help out with this module too. I've been a huge BFRPG fan for years.
@Github-Reneon How far did you get with the v12 compatibility? If we combine our efforts then it's going to be more productive than going at this individually. I did install and test the current state of your fork about a week ago, but it didn't appear to be working in v12 at the time.
I just made a fork of my own. I'm going to start looking at this issue in particular, since it looks like the most critical bug in v12 so far.
DC23
commented
3 weeks ago Ok, I've been working with the helpful community at the Foundry Discord and I think I know what's going on. Will test tomorrow and if it works then should have a PR in a few days for this issue.
DC23
commented
2 weeks ago I've identified the issue as relating back to Foundry data model changes introduced all the way back in version 10, specifically changes to the semantics for the inner object of system-specific data. They removed an inner object called data and renamed from data to system. Apparently, in v10 they put backwards compatibility support in place for systems that continued to reference actor.data.data instead of actor.system but in v12 that backwards compatibility has been dropped.
I suspect that the ActorSheet class cloning the data to the top level of the context as context.data for easier access has been masking this issue somewhat. By reading the values from here, the character sheets continue to show the default values. I've temporarily added a log statement to the character sheet template to see the context which lets me know the correct input element names to use for Foundry serialisation to work in v12. I'll leave the cloned data at context.data alone, since I want to make as few changes as possible, though it does lead to confusing lines in the template, such as
<input type="text" name="system.hitPoints.value" value="{{data.hitPoints.value}}" data-dtype="Number"/>where the name is using the newer system.hitPoints.value, and the display value looks like it's still using the older approach. I could change label to {{document.system.hitPoints.value}} which is more verbose and less fragile, but I'll hold off on that for now. My goal is the smallest number of changes needed to get the module working for v12.
orffen
commented
2 weeks ago Thanks for your work on this!
DC23
commented
2 weeks ago No problem. I'd been meaning to get to it for a while, but I had to learn JS and the Foundry API first. Spent the last month or so doing both :)
I'm steadily working through fixing and testing all the templates now. Should have a PR ready in a day or so for the bug. I'm not sure whether it's better to submit a PR just for this bug, or to lump it into a complete v12 compatibility PR. I guess that depends on whether anything else is required for v12 beyond the system.json updates.
DC23
commented
2 weeks ago just noting so I don't forget that fields in items are affected as well. For example, adding @str.bonus to the damage roll on a weapon is not saved. I'll fix this too as part of the PR. I just want to write it down so I don't forget :)
My commit to the item template only addressed the currency fields on the equipment tab of the character sheet, not the dialog for editing items. Still working through all the templates.
DC23
commented
2 weeks ago Huh, while testing I realised that the special abilities had the same issue and went to look for the template. It took a minute before I realised they are in the core rules module. At least I won't have to redo the detective work on that. Once we get the system updated to v12, I'll see if I can help get the module updated to v12. Not relevant here
DC23
commented
2 weeks ago Huh, while testing I realised that the special abilities had the same issue and went to look for the template. It took a minute before I realised they are in the core rules module. At least I won't have to redo the detective work on that. Once we get the system updated to v12, I'll see if I can help get the module updated to v12. Not relevant here
This is incorrect. While the ability came from the module, the edit functions came from the system. I just hadn't fixed that template yet. I have now and it's working.
I need more testing, but I can't actually see any more issues in v12 at the moment. I'll submit a v12 compatibility PR that combines this bug fix into it.
Github-Reneon
commented
2 weeks ago Dude @DC23 you're absolutely amazing
DC23
commented
2 weeks ago Dude @DC23 you're absolutely amazing
Thanks 😀
Description
In Foundry VTT version 12, when updating values in actor sheets (such as hit points, abilities, or custom attributes), the changes do not persist. After changing a value and then focusing on another input field or clicking away, the value reverts to its previous state instead of saving. This issue was not present in v11 and continues to not be present, and appears to affect all actor sheets in this system.
Steps to Reproduce
Expected Behavior
Values entered in the actor sheet should persist and update the actor's data when the user changes the field's value and clicks away or focuses on another field.
Actual Behavior
After modifying a field, the value reverts to its original state when focus is lost or the user interacts with another element on the sheet.
System Information