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 a Migration system and fix #172 #184

Merged
merged 17 commits into from
Feb 15, 2024
Merged

Conversation

Eryx5502
Copy link
Contributor

@Eryx5502 Eryx5502 commented Oct 31, 2023

To solve #172, a migration of the actor data needs to be performed. This led me to introduce a Migration system heavily inspired by Pathfinder's one, yet simplified and applied to our case.

A documentation page (both in Spanish and English has been added, explaining in detail how the system works and how to write new migrations. I didn't feel able to write also the French translation (although I'll be happy to review it if @LIARDM has time for translating).

The functions migrating items and tokens need still to be written, but I thought it's ok to do that when we have a particular migration for them on mind. Also, the ability to migrate actor data is needed for PR #181, which introduces some urgency.

Finally, a migration is added to reassign the properties system.general.settings.fumbles.value and the system.general.settings.openRolls.value, solving the original issue.

@LIARDM
Copy link
Contributor

LIARDM commented Nov 1, 2023

Hello !
I tried to bring a traduction since I'm in holydays ! 🙂
This time my changes are submitted with my MR directly in yours, I hope it won't be an hinderance.

LIARDM and others added 2 commits November 2, 2023 22:47
Yes, right ! I did'nt thought of it this way. :)

Co-authored-by: Aitor Balmaseda <32964371+Eryx5502@users.noreply.github.com>
documentation FR pour migrations.md
Copy link
Contributor

@Linkaynn Linkaynn left a comment

Choose a reason for hiding this comment

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

Disculpa la tardanda @Eryx5502. ¡Esto es una maravilla! Aprobado

Comment on lines +18 to +22
if (game.settings.get('animabf', ABFSettingsKeys.DEVELOP_MODE)) {
console.warn(
`AnimaBF | Migration ${migration.version} needs not to be applied, current system migration version is ${currentVersion}.`
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

src/packs/npcs/LOG Outdated Show resolved Hide resolved
@Linkaynn
Copy link
Contributor

Lo unico que me "preocupa" son los ficheros que están en packs/npcs/LOG por si me puedes confirmar si son necesarios @Eryx5502

There was a problem getting the updateData with `.toObject()` from the modified actor.
When modifying the actor inplace without calling `.update(...)` the actor source (the Document DataSource) is not updated, and `.toObject()` returns the old `actor.system`.
@Eryx5502 Eryx5502 force-pushed the fix/172-enable-fumble-by-default branch from fc496bb to cd6339a Compare February 7, 2024 19:42
@Eryx5502 Eryx5502 merged commit 28046a9 into main Feb 15, 2024
@Eryx5502 Eryx5502 deleted the fix/172-enable-fumble-by-default branch February 15, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants