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.

Sign up for GitHub

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

Jump to bottom

Trim down migration runner handling of synthetic actors & AEs #8363

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Trim down migration runner handling of synthetic actors & AEs #8363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 7 additions & 16 deletions src/module/migration/runner/base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,14 @@ export class MigrationRunnerBase {
return currentVersion < (this.constructor as typeof MigrationRunnerBase).LATEST_SCHEMA_VERSION;
}

diffCollection<T extends foundry.documents.ActiveEffectSource>(orig: T[], updated: T[]): CollectionDiff<T>;
diffCollection<T extends ItemSourcePF2e>(orig: T[], updated: T[]): CollectionDiff<T>;
diffCollection<T extends foundry.documents.ActiveEffectSource | ItemSourcePF2e>(
orig: T[],
updated: T[]
): CollectionDiff<T>;
diffCollection<TSource extends foundry.documents.ActiveEffectSource | ItemSourcePF2e>(
orig: TSource[],
updated: TSource[]
): CollectionDiff<TSource> {
const ret: CollectionDiff<TSource> = {
diffCollection(orig: ItemSourcePF2e[], updated: ItemSourcePF2e[]): CollectionDiff<ItemSourcePF2e> {
const diffs: CollectionDiff<ItemSourcePF2e> = {
inserted: [],
deleted: [],
updated: [],
};

const origSources: Map<string, TSource> = new Map();
const origSources: Map<string, ItemSourcePF2e> = new Map();
for (const source of orig) {
origSources.set(source._id!, source);
}
Expand All @@ -55,21 +46,21 @@ export class MigrationRunnerBase {
if (origSource) {
// check to see if anything changed
if (JSON.stringify(origSource) !== JSON.stringify(source)) {
ret.updated.push(source);
diffs.updated.push(source);
}
origSources.delete(source._id!);
} else {
// it's new
ret.inserted.push(source);
diffs.inserted.push(source);
}
}

// since we've been deleting them as we process, the ones remaining need to be deleted
for (const source of origSources.values()) {
ret.deleted.push(source._id!);
diffs.deleted.push(source._id);
}

return ret;
return diffs;
}

async getUpdatedActor(actor: ActorSourcePF2e, migrations: MigrationBase[]): Promise<ActorSourcePF2e> {
Expand Down
97 changes: 19 additions & 78 deletions src/module/migration/runner/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class MigrationRunner extends MigrationRunnerBase {
const updated =
"items" in document
? await this.#migrateActor(migrations, document, { pack })
: await this.#migrateItem(migrations, document, { pack });
: await this.#migrateItem(migrations, document);
if (updated) updateGroup.push(updated);
}
if (updateGroup.length > 0) {
Expand All @@ -83,41 +83,16 @@ export class MigrationRunner extends MigrationRunnerBase {
}
}

async #migrateItem(
migrations: MigrationBase[],
item: ItemPF2e,
options: { pack?: string | null } = {}
): Promise<ItemSourcePF2e | null> {
const { pack } = options;
async #migrateItem(migrations: MigrationBase[], item: ItemPF2e): Promise<ItemSourcePF2e | null> {
const baseItem = item.toObject();
const updatedItem = await (() => {
try {
return this.getUpdatedItem(baseItem, migrations);
} catch (error) {
if (error instanceof Error) {
console.error(`Error thrown while migrating ${item.uuid}: ${error.message}`);
}
return null;
}
})();
if (!updatedItem) return null;

const baseAEs = [...baseItem.effects];
const updatedAEs = [...updatedItem.effects];
const aeDiff = this.diffCollection(baseAEs, updatedAEs);
if (aeDiff.deleted.length > 0) {
try {
const finalDeleted = aeDiff.deleted.filter((deletedId) =>
item.effects.some((effect) => effect.id === deletedId)
);
await item.deleteEmbeddedDocuments("ActiveEffect", finalDeleted, { noHook: true, pack });
} catch (error) {
console.warn(error);
try {
return this.getUpdatedItem(baseItem, migrations);
} catch (error) {
if (error instanceof Error) {
console.error(`Error thrown while migrating ${item.uuid}: ${error.message}`);
}
return null;
}

updatedItem.effects = item.actor?.isToken ? updatedAEs : aeDiff.updated;
return updatedItem;
}

async #migrateActor(
Expand All @@ -140,63 +115,29 @@ export class MigrationRunner extends MigrationRunnerBase {
})();
if (!updatedActor) return null;

if (actor.effects.size > 0) {
// What are these doing here?
actor.deleteEmbeddedDocuments("ActiveEffect", [], { deleteAll: true });
}

const baseItems = [...baseActor.items];
const baseAEs = [...baseActor.effects];
const updatedItems = [...updatedActor.items];
const updatedAEs = [...updatedActor.effects];

// We pull out the items here so that the embedded document operations get called
const itemDiff = this.diffCollection(baseItems, updatedItems);
if (itemDiff.deleted.length > 0) {
const finalDeleted = itemDiff.deleted.filter((id) => actor.items.has(id));
if (finalDeleted.length > 0) {
try {
const finalDeleted = itemDiff.deleted.filter((deletedId) =>
actor.items.some((item) => item.id === deletedId)
);
await actor.deleteEmbeddedDocuments("Item", finalDeleted, { noHook: true, pack });
} catch (error) {
// Output as a warning, since this merely means data preparation following the update
// (hopefully intermittently) threw an error
console.warn(error);
}
}
const finalUpdated = itemDiff.updated.filter((i) => actor.items.has(i._id));
updatedActor.items = [...itemDiff.inserted, ...finalUpdated];

const aeDiff = this.diffCollection(baseAEs, updatedAEs);
if (aeDiff.deleted.length > 0) {
try {
const finalDeleted = aeDiff.deleted.filter((deletedId) =>
actor.effects.some((effect) => effect.id === deletedId)
);
await actor.deleteEmbeddedDocuments("ActiveEffect", finalDeleted, { noHook: true, pack });
} catch (error) {
console.warn(error);
}
}

if (itemDiff.inserted.length > 0) {
try {
await actor.createEmbeddedDocuments("Item", itemDiff.inserted, { noHook: true, pack });
} catch (error) {
console.warn(error);
}
}

// Delete embedded ActiveEffects on embedded Items
for (const updated of updatedItems) {
const original = baseActor.items.find((i) => i._id === updated._id);
if (!original) continue;
const itemAEDiff = this.diffCollection(original.effects, updated.effects);
if (itemAEDiff.deleted.length > 0) {
// Doubly-embedded documents can't be updated or deleted directly, so send up the entire item
// as a full replacement update
try {
await Item.updateDocuments([updated], { parent: actor, diff: false, recursive: false, pack });
} catch (error) {
console.warn(error);
}
}
}

updatedActor.items = actor.isToken ? updatedItems : itemDiff.updated;
return updatedActor;
}

Expand Down Expand Up @@ -315,7 +256,7 @@ export class MigrationRunner extends MigrationRunnerBase {
});

// Migrate World Actors
await this.#migrateDocuments(game.actors as WorldCollection<ActorPF2e<null>>, migrations, progress);
await this.#migrateDocuments(game.actors, migrations, progress);

// Migrate World Items
await this.#migrateDocuments(game.items, migrations, progress);
Expand Down Expand Up @@ -369,7 +310,7 @@ export class MigrationRunner extends MigrationRunnerBase {
const updated = await this.#migrateActor(migrations, actor);
if (updated) {
try {
await actor.update(updated);
await actor.update(updated, { noHook: true });
} catch (error) {
console.warn(error);
}
Expand Down
9 changes: 7 additions & 2 deletions tests/mocks/actor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,22 @@ export class MockActor {
_context: DocumentModificationContext<TokenDocumentPF2e<ScenePF2e | null>> = {}
): Promise<ActorPF2e[]> {
return updates.flatMap((update) => {
const actor = game.actors.find((actor) => actor.id === update._id);
const actor = game.actors.find((a) => a.id === update._id);
if (!actor) throw Error("PANIC!");

const itemUpdates = (update.items ?? []) as DeepPartial<ItemSourcePF2e>[];
delete update.items;
mergeObject(actor._source, update);
for (const partial of itemUpdates) {
partial._id ??= "item1";
const source = actor._source.items.find(
(maybeSource: ItemSourcePF2e) => maybeSource._id === partial._id
);
if (source) mergeObject(source, partial);
if (source) {
mergeObject(source, partial);
} else {
actor.createEmbeddedDocuments("Item", [partial]);
}
}
actor.prepareData();
return actor;
Expand Down
3 changes: 1 addition & 2 deletions tests/module/migration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,7 @@ describe("test migration runner", () => {
static version = 14;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
async updateActor(actor: { items: any[] }) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
actor.system.sampleItemId = actor.items.find((x: any) => x.name === "sample item")._id;
actor.system.sampleItemId = actor.items.find((i: { name: string }) => i.name === "sample item")._id;
}
}

Expand Down