Refactor `ChatMessagePF2e` flags

[In3luki]

Deprecations: ChatMessagePF2e#flags#pf2e#appliedDamage moved to ChatMessagePF2e#system#appliedDamage ChatMessagePF2e#flags#pf2e#origin moved to ChatMessagePF2e#system#origin and refactored to:

interface ChatMessageOriginData {
    actor: ActorUUID | TokenDocumentUUID | null;
    token?: TokenDocumentUUID;
    item?: ItemUUID;
    rollOptions?: string[];
    /** Is the origin the roller? */
    self?: boolean;
}

ChatMessagePF2e#flags#pf2e#context#target refactored to:

interface ChatMessageTargetData {
    actor?: ActorUUID | TokenDocumentUUID | null;
    token?: TokenDocumentUUID;
    item?: ItemUUID;
    distance: number | null;
    rangeIncrement: number | null;
}

ChatMessagePF2e#flags#pf2e#context moved to ChatMessagePF2e#system#context and partially refactored.

~~I've opted to move origin to top level system data to avoid having to add bogus origins to context data that has no origin.~~

interface ChatMessageSystemData {
    /** A record of applied damage that can be undone */
    appliedDamage?: AppliedDamageData | null;
    /** Message context data that describes the type of this chat message */
    context?: ChatMessageContext;
}

Old messages still work thanks to DataModel#migrateData.

[CarlosFdez]

Why is this v12? These are pf2e flags.

[In3luki]

ChatMessages don't have system data in v11.

[CarlosFdez]

I double checked, and at least as far as context data is concerned, I suppose this is symmetrical with effects. I'll need to compare more closely.

[CarlosFdez]

Looking deeper, I think being able to move them is good, but I don't think it can be 1-1. Some of these properties should probably be nested (maybe origin should be in context, unsure) or still contained in flags (journalEntry?).

I'll do a quick runthrough of v12 to see if we're missing anything, but otherwise I'll try to think of a new structure.

[CarlosFdez]

So off the top of my head, very quickly (do not act on this, I'm currently speculating, I have very little knowledge of the chat pipeline):

modifiers, dice, origin (which is really just the origin's item) and casting should probably merged into context.

appliedDamage can stay where it is, its fine.

I have no idea about journalEntry. Maybe its context.

preformatted and suppressDamageButtons should probably remain as flags. Maybe. I'm super uncertain about this, but maybe suppressDamageButtons can be moved somewhere.

[CarlosFdez]

I went over the code, and I'm kinda torn between one of these two general structures. I'd like some more opinions on what sort of approach we should go for before doing we do anything (note: they're merged and flattened and ignores potential improvements like discriminated unions. I did it like that so it can be viewed at a glance).

https://gist.github.com/CarlosFdez/6269e76ab8932d77558a9783dd9a5c44

The idea is that type should establish a discriminated union. Then certain ones will have certain properties. Of note is that the spellcasting stuff actually shouldn't be specific to spell-cast. A spell attack roll could still have info on the spell itself, even if we don't fill it out yet.

I think because this is V12, we don't need to shim the old code. Migration only should suffice.

[In3luki]

I would prefer Choice A. Having the source item information, like the variant and rank, under item makes more sense than dumping them directly into origin IMO.

[CarlosFdez]

Choice A is where I'm leaning. Because when unserialized, all that information folds into a new ItemPF2e. You can get the rank from the spell document itself for example. And you need the rank/variants to recreate the spell from the chat message. Translates 1-1.

Granted, I could be wrong about a few of the props. For example, I'm uncertain about altUsage (and only slightly uncertain about collectionId). But I'm pretty confident on the others.

I presented choice B because I could see some people prefer the flattened structure though, 🤷.

[CarlosFdez]

This got absolutely demolished by the rebase, but it already had some merge conflicts anyways so it might be worth redoing.