Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add default settlement data to "add-settlement" handler in kingdom sheet #18134

Closed
wants to merge 1 commit into from

Conversation

MaximMukhanov
Copy link

Prevents kingdom sheet crash on adding settlement due to the lack of settlement data in "add-settlement" handler.

foundryvtt/foundryvtt-premium-content#898 - issue in foundry premium modules repo that explains the problem.

…eet to avoid sheet crashing due to missing consumption value
@MaximMukhanov MaximMukhanov changed the title Adds default settlement data to "add-settlement" handler in kingdom sheet Add default settlement data to "add-settlement" handler in kingdom sheet Jan 28, 2025
@CarlosFdez
Copy link
Collaborator

I'll dig into this soon and see if we can't solve this by using default values, but if I can't get something clean I'll simply merge this in. Thank you for looking into it.

@MaximMukhanov
Copy link
Author

No problem, thank you for working on system!

@CarlosFdez
Copy link
Collaborator

CarlosFdez commented Jan 30, 2025

From my initial investigation, this looks to be a bug with initial values of schema fields used in RecordField. Putting the schema field top level initializes all default values correctly, but it breaks apart entirely in the RecordField.

EDIT: this may be wrong, it may be data prep related

@MaximMukhanov
Copy link
Author

MaximMukhanov commented Jan 30, 2025

From my initial investigation, this looks to be a bug with initial values of schema fields used in RecordField. Putting the schema field top level initializes all default values correctly, but it breaks apart entirely in the RecordField.

EDIT: this may be wrong, it may be data prep related

Also dug a bit on my lunch break today:D

It seems that "prepareBaseData" method incorrectly handles lack of settlement data provided by the handler.

for (const settlement of settlements) {
            if (!settlement) continue; // this line seems to be the culprit
            const typeData = KINGDOM_SETTLEMENT_TYPE_DATA[settlement.type];
            settlement.consumption.base = typeData.consumption;
            settlement.consumption.total = Math.max(0, typeData.consumption - settlement.consumption.reduction);
        }

Correct me if I'm wrong

@CarlosFdez
Copy link
Collaborator

CarlosFdez commented Jan 30, 2025

stwlam did a fix last night, is this still an issue? Its because the actual data prep does a few extra things to lead to default values (clean and reset).

This was pre fix:
image

@CarlosFdez
Copy link
Collaborator

I tested and the bug seems to be resolved now. Thanks a lot for attempting to fix this, and for bringing this bug to our attention.

@CarlosFdez CarlosFdez closed this Jan 31, 2025
@MaximMukhanov
Copy link
Author

I tested and the bug seems to be resolved now. Thanks a lot for attempting to fix this, and for bringing this bug to our attention.

Thank you for your work:D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants