[Refactoring]: Token Split / Refactoring

[cwisniew]

Describe the problem

The way that tokens are currently implemented by a single Token class that belongs to a Zone create some limitations and is beginning to cause some issues.

The biggest limitation of this design is that all tokens are completely seperate so if you have a token on multiple maps that is the same character changes to one of them will not be reflected in the other. This can require a lot of bookkeeping or macros and Lib: tokens to work around.

The two biggest issues that this causes is increase in memory use for tokens that store a large amount of data and increased time that MapTool is paused when autosaving. While the increase in memory usage may not yet be significant (it's hard to tell without detailed stats that we don't collect), @bubblobill has run into a problem where his MapTool will pause for over 15 seconds to perform autosaving on one of this campaigns. This occurs because a deep clone is made of the Zones on the GUI thread at the start of the autosave process

The improvement you'd like to see

Deprecate usage of the Token class in the code and create two new classes to deal with its functionality. MapToken which is the representation of a Token on a Map (including things tied to a map such as initiative) and GameCharacter which is the representation of all the other stuff for character.

The MapToken would continue to be saved in the current way but since there is much less data associated with it this will help alleviate the current issue -- Although it is still possible to run into the problem with a large number of tokens, but fixing that is outside the scope of this change.

The GameCharacter information will be stored in a thread safe way. Then as part of the background process this information will be saved to the campaign so that it does not pause the GUI thread. This is similar to what happens for add-ons and GameData at the moment.

There is a small downside to saving CharacterData this way, as currently if a macro starts an update then the macro must complete before the cloning of all data in tokens can be performed and this blocks any macro updates and any updates on the GUI thread from making changes until that in memory copy has completed. With the change it will be possible for data to be saved as it is mid macro meaning a macro started changing one property but the data was written to the autosave before it updated a second property in the same macro. Given that macros make no guarantees of atomicity, the current solution will also suffer similar issues the advantages should out weigh the problems.

A first pass of the Token class would suggest the following break up

MapToken would retain the required constants and the current fields from Token (this would be the perfect opportunity to rename a few of them though)

• id
• exposedAreaGUID
• TokenShape
• x, y, z
• anchorX, anchorY
• sizeScale = 1
• lastX, lastY
• lastPath
• snapToScale
• width, height, isoWidth, isoHeight
• scaleX, scaleY
• sizeMap
• snapToGrid
• isVisible
• visibleOnlyToOwner
• vblColorSensitivity
• alwaysVisibleTolerance, isAlwaysVisible
• vbl, hillVbl, pitVbl, coverVbl, mbl
• tokenShape
• tokenType
• layer
• actualLayer
• facing
• haloColorValue, haloColor
• visionOverlayColorValue, visionOverlayColor
• tokenOpacity
• TerrainModifierOperation
• terrainModifier
• terrainModifierOperation
• terrainModifiersIgnored
• isFlippedX, isFlippedY, isFlippedIso
• uniqueLightSources, lightSourceList
• sightType, hasSight
• hasImageTable, imageTableName
• label
• state
• charsheetImage, portraitImage

GameCharacter would then retain the following information

• statSheet
• strictGsonObjectAdapter
• imageAssetMap
• currentImageAsset
• name
• ownerList, ownerType
• propertyType
• speechName
• notes, notesType, gmNotes, gmNoteType
• gmName
• propertyMap
• macroMap
• macroPropertiesMap
• speechMap
• heroLabData
• allowURIAccess

Missing from this list is isBeingImpersonated as it requires a bit of investigation (its rarely used so can we get away with just not having it).

This break up is based on the current property names of the classes and wont necessarily be the new names for example notes could be more generic, e.g. something like this

public record GameNotes(String label, SyntaxConstants syntaxType, String notes)

public class GameCharacter {
    // ... Stuff here
    private final Map<String /* label */, GameCharacter> notes = new HashMap<>();

    public GameNotes getNotes(String label) {
        return notes.get(label);
    }

    public void setNotes(GameNotes n) {
        notes.put(n.label(), n);
    }
}

And there are plenty of other improvements that can be made like this since its the perfect time to do so when introducing the class.

Ways to extend this functionality in the future

• [ ] Ability to edit GameCharacter outside of editing a MapToken
• [ ] Ability to designate GameCharacter as one of
  • [ ] Standard, every MapToken linked to a GameCharacter shares all data with all other MapTokens linked to it
  • [ ] Copy on Write, works like Standard, but new values are created when updated (useful for minions/unnamed NPCs)
  • [ ] Templates, like Copy on Write, but when first accessing a property it will "execute" the property to come up with a value (great for cutting and pasting all your goblin minions with generated hit points)

Expected Benefits

• The critical part of the autosave process where it causes the GUI to pause is reduced
• More flexibility for managing tokens and their data
• one small step on the path to future benefits such as on-demand/caching access to properties

Additional Context

This will be a large change as it will require modifying how campaigns are persisted. The current Token class would be retained but deprecated, the Token lists in Zone s will be retained but deprecated and made transient, when a campaign is loaded from the campaign file at the end of the loading if these lists are not empty then it will convert them to the new MapToken and GameCharacter classes and clear out the Token list.

This is one step down the path to fixing the campaign persistence issues. Ideally with proper caching implemented only the frequently/recently used properties on tokens will be in memory. While this is far off in the future it will at least bring a lot of token (and lib:token) data to the same level as GameData and add-on static data which are already geared to take advantage of this when available.

[thelsing]

I like this idea. The name GameCharacter seems to imply that it's only for characters. Maybe something like GameEntity would be better?

[cwisniew]

I like this idea. The name GameCharacter seems to imply that it's only for characters. Maybe something like GameEntity would be better?

Entity would imply its for anything (not just tied to a token though, Entity is almost like object where everything is an Entity), I agree the name is not great though, but I haven't been able to think of a better one :(

[thelsing]

As token may represent anything like lights, locations, buildings, notes, spell effects, terrain, etc. entity might fit well.

[cwisniew]

As token may represent anything like lights, locations, buildings, notes, spell effects, terrain, etc. entity might fit well.

I guess we could do MappableEntity as it only works for things on the map hmm that name is not great either let me mull on it a while :)

[ColdAnkles]

I'd like to be able to set MapToken properties on a GameEntity (like vbl, shape, sight, lightsources) so requesting a new MapToken based on a GameEntity shares those things.

And I think states and sight are more appropriate for GameEntity since they aren't likely to change because of changes in map

[cwisniew]

I'd like to be able to set MapToken properties on a GameEntity (like vbl, shape, sight, lightsources) so requesting a new MapToken based on a GameEntity shares those things.

All these things are map based, the GameCharacter would have them to be inherited by the token when create a new token, but then they belong to the token (which overrides the GameCharacter) especially in the case of VBL and shape which are token characteristics.

And I think states and sight are more appropriate for GameEntity since they aren't likely to change because of changes in map

State is a difficult one as for an NPC it means you will be resetting state when you move to another map (i.e. everyone knows you HAVE to fight the big bad guy more than once... 1/2 year later they are unlikely to still be bloodied). We can move it to the GameCharacter but I am sure someone is going to be disappointed either way :)

[ColdAnkles]

All these things are map based, the GameCharacter would have them to be inherited by the token when create a new token, but then they belong to the token (which overrides the GameCharacter) especially in the case of VBL and shape which are token characteristics.

Even if they're defaults/initial settings would be better than nothing - MapToken could take optional list of settings for when a GameCharacter is requested to be instantiated.

State is a difficult one as for an NPC it means you will be resetting state when you move to another map (i.e. everyone knows you HAVE to fight the big bad guy more than once... 1/2 year later they are unlikely to still be bloodied). We can move it to the GameCharacter but I am sure someone is going to be disappointed either way :)

It's always going to be a personal preference, but they way I see it is that PCs will be the ones most commonly existing across multiple maps - going from one map to the next means spawning new PC mapTokens, then setting any states that should still apply and anything else. and clear all states is an option, whereas set states identically to X token isn't.

[bubblobill]

I would like to see tokens as a collection of classes. Or more accurately, a collection of ids to class instances.

Working on fancy halos I created a Halo class then shoehorned it into Token.

What I should have done is kept a cache of halo instances and stored the UID on Token.

TBH I found extending Token functionality a right pain. Many of the properties should probably be split to other classes, even if they are placeholders, to ease the way for extension.

On the semantic argument, this is RPTools, stick with character or go tokenData. I want to get away from PC/NPC to allow teams. This would be a good opportunity to start loosening the hard-wiring of such concepts.

[bubblobill]

So, I see this as a split between/into a render object and a data object. With a relationship of one data to (potentially) many drawn.

I'm not 100% on the pros and cons, but I would like to be able to attach things to tokens dynamically and get away from the monolithic token class. I'm not knowledgeable enough to know what structures would work best and be the most flexible for tacking on future improvements. What approaches would provide flexibility and allow new contributors to grasp how to use it?

[cwisniew]

I'm not 100% on the pros and cons, but I would like to be able to attach things to tokens dynamically and get away from the monolithic token class. I'm not knowledgeable enough to know what structures would work best and be the most flexible for tacking on future improvements. What approaches would provide flexibility and allow new contributors to grasp how to use it?

Attach what type of things?

[bubblobill]

Attach what type of things?

Example: Halo class that supports images, shapes,etc. I found adding a new class to Token to be quite painful.

Extending layout features was no picnic. If the token class held an instance of a location class, and an instance of a display class it would be easier (I think) to improve those classes or extend them.

A background object does not necessarily need to see or hear things. So it would not need a sense class.

Rather than have PC/NPC, have an affiliation class that can be extended to allow teams.

[cwisniew]

Attach what type of things?

Example: Halo class that supports images, shapes,etc. I found adding a new class to Token to be quite painful.

Halos we can do but it might (probably means it won't be fully supported in MTscript

Extending layout features was no picnic. If the token class held an instance of a location class, and an instance of a display class it would be easier (I think) to improve those classes or extend them.

We are somewhat constrained with not being able to break MTScript. Other than that you will have to provide more details on the problems because as described I don't think the problem is with how the token class is organised. The token will be the "the display class" and location values vs a class to hold those doesn't change much, the location class can't be responsible for much because we still have to render efficiently asking each token/stamp to position itself won't help achieve this

A background object does not necessarily need to see or hear things. So it would not need a sense class.

MTscript has a different opinion on this :( also things can freely be moved between layers

Rather than have PC/NPC, have an affiliation class that can be extended to allow teams.

This has nothing to do with attaching things to tokens though

[bubblobill]

Doesn't help that I don't know the language to effectively describe what I'm asking :)

I'll try a different tack. Messing with the Token class is hard. I want to add/modify a feature. There's the DTO, the command list, action/server commands, and so on with so many ways to screw up. You can't just add a class and point things at it, with overrides for the bits you are replacing.

I want all this stuff to somehow be easier, somehow.

[bubblobill]

On the location class. It limits you. A location class could contain a 3D point, a vector relating it to its footprint, and be extensible to a volume or 3D object in future.

[cwisniew]

Doesn't help that I don't know the language to effectively describe what I'm asking :)

I'll try a different tack. Messing with the Token class is hard. I want to add/modify a feature. There's the DTO, the command list, action/server commands, and so on with so many ways to screw up. You can't just add a class and point things at it, with overrides for the bits you are replacing.

I want all this stuff to somehow be easier, somehow.

You won't avoid any of that stuff, in fact if you went to override classes you will need to do extra work for dto, and there will be more ways to screw up:)

[bubblobill]

I have been having an issue with this concept for a while without being able to articulate it. I think I have worked out that the problem is in the rendering info side of the split.

Since this will set the path of future development we should be doing it in a way that makes pie-in-the-sky ideas possible, for example; tokens in a 3D environment, or moving grids to sub-classes of gridless. Much of the hard-coded render information is restrictive and inconsistent between differing implementations.

I would like to see much of the data more compartmentalised and better categorised. Something like;

Spatial Data

• Location in co-ordinate space, e.g. [x, y, z, w]
• Transforms, i.e. translation, scale(flipping & scaling), skew(iso-pain), and facing
• Volume, i.e. Token Footprint, 3D mesh
• Centre of rotation
• etc.,

Computational Data

• Terrain cost
• Sight
• Light
• VB

Features;

• Image table
• States
• Halos