Token Footprint Functions and Campaign Properties

[ColdAnkles]

Identify the Bug or Feature request

Basic Implementation of #4849, submitted for review

Description of the Change

Moves Token Footprints from being Static and Part of Campaign Properties.
MTScript functions for getting and setting Footprints

Possible Drawbacks

Initial naive implementation, possibly with some weirdness from my dev environment

Documentation Notes

getTokenFootprints()
getTokenFootprints(gridType)
getTokenFootprints(gridType, footprintName)

getGridTypes()

getFootprintNames()
getFootprintNames(gridType)

removeTokenFootprint(gridType, footprintName)

setTokenFootprint(gridType, footprintName, JSONFootPrintData)

resetFootprintsToDefault()

Release Notes

Custom Token Footprint Support

---

This change is 

[cwisniew]

This really needs a UI, functionality like this shouldn't be locked behind macros, also going by past experience macro only functions don't get tested until its too late to change anything if its broken,

[cwisniew]

This really needs a UI, functionality like this shouldn't be locked behind macros, also going by past experience macro only functions don't get tested until its too late to change anything if its broken,

[cwisniew]

The tests this fails will also need to be fixed up

[ColdAnkles]

Agreed on the UI front, but it's not something I'm very good at, so since @bubblobill offered to do the GUI in #4849 if I got the backend, this PR is solely for backend enablement of the incoming GUI portion.
The macro parts were mostly because I'm capable of implementing them, and I needed something to help test with. (and they'd be desired functionality anyway).
I also added a quick and dirty mitigation for #4856 - though would like something better (as in generating a custom footprint that appears to cover the approximate area). Needs more thought.

On the tests failing: I've taken a look and can't see anything obvious between what I've changed and which tests fail

[cwisniew]

On the tests failing: I've taken a look and can't see anything obvious between what I've changed and which tests fail

Taking a quick look my guess would be the calls to CampaignProperties.loadFootprints(), the XML probably not loading correctly as its getting xstream errors. Try remove the localizedName in the classe (which shouldn't be saved anyway) and see if that fixes the issue. Or if you need that variable put transient in front of it so it is not persisted

you can run the tests locally with .\gradlew.bat test. (or ide) to test

[bubblobill]

I noticed you didn't include the grid type "isometric hex". One day someone is going to implement it beyond including it in the code add as an option. :)

[cwisniew]

I noticed you didn't include the grid type "isometric hex". One day someone is going to implement it beyond including it in the code add as an option. :)

Thanks for volunteering... Just know I believe in you @bubblobill :)

[ColdAnkles]

On the tests failing: I've taken a look and can't see anything obvious between what I've changed and which tests fail

Taking a quick look my guess would be the calls to CampaignProperties.loadFootprints(), the XML probably not loading correctly as its getting xstream errors. Try remove the localizedName in the classe (which shouldn't be saved anyway) and see if that fixes the issue. Or if you need that variable put transient in front of it so it is not persisted

you can run the tests locally with .\gradlew.bat test. (or ide) to test

Okay, its definitely the footprint loading - removing the localizedName variable or making it transient didn't make a difference, but omitting all the calls to footprint loading from the initFootprints step did. Not sure what to try next.

At any rate: the reason for my choices with localizedName existing separate to name is to allow for a framework to define sizes/footprints and have the display/localized name overwritten freely by someone else without effecting ability to address footprints by their "true" name - and also not having to make said localization dependant on MapTool - though it does work on the assumption that servers and clients are working in the same locale.
Also it doesn't work as intended yet.

[cwisniew]

Okay, its definitely the footprint loading - removing the localizedName variable or making it transient didn't make a difference, but omitting all the calls to footprint loading from the initFootprints step did. Not sure what to try next.

There is probably no good reason to have default footprints in external serialised files in any case, probably just make classes with the default values

At any rate: the reason for my choices with localizedName existing separate to name is to allow for a framework to define sizes/footprints and have the display/localized name overwritten freely by someone else without effecting ability to address footprints by their "true" name - and also not having to make said localization dependant on MapTool - though it does work on the assumption that servers and clients are working in the same locale. Also it doesn't work as intended yet.

This is not a hreat assumption though there will be cases where client is different from server. Really it should be something that is localised within MT and users don't need to localise themselves

[ColdAnkles]

This is not a hreat assumption though there will be cases where client is different from server. Really it should be something that is localised within MT and users don't need to localise themselves

including localizations for every possible word/phrase people might choose to name a token size?

[cwisniew]

This is not a hreat assumption though there will be cases where client is different from server. Really it should be something that is localised within MT and users don't need to localise themselves

including localizations for every possible word/phrase people might choose to name a token size?

Ah gotchya, it's a super supplied name? Then that is fine

[ColdAnkles]

Okay, so excising the XML based defaults and replacing with code based works fine and tests pass.

However I've noticed that some footprints on hex grids are busted, and I can't see how my changes effect this

[bubblobill]

There are about 5 principal methods of referencing grids, each with a zillion variants. The one we use is probably the least useful for compatibility and interoperability between grid types with the possible exception of polar coordinates.
Every time I considered doing what you are doing, I baulked because the xml makes no sense.

[ColdAnkles]

Here's what I'm seeing re: Hex Grid weirdness

Standard Large footprint on both hex grids do this kind of thing, yet the coordinates in the footprint are unchanged

[bubblobill]

Didn't someone recently fix something relating to footprints? Maybe in #4830?

[ColdAnkles]

It's definitely something related to my changes - develop and 1.14.3 don't have the issue

[bubblobill]

Swap your vertical and horizontal hex logic.

[ColdAnkles]

I propose footprints contain the following data;

1. The cell from which vision is determined.

2. The cell from which light is calculated.

3. A point (not cell) on which rotation occurs, assumed to be the origin cell centre unless defined.

I did some quick tests - looks like they originate from the point at the centre of the relative x/y rectangle bounding box.

Fundamentally I agree, but unsure if I want to get into extending footprints with that functionality at this point.

At any rate, I solved my offset grid issues and will open a new issue proposing changes to hex grid coordinate systems.

[bubblobill]

Minor annoyance - you cannot set the canonical name for a footprint, only the localised name.
This is fine for people doing it for themselves, but annoying for wanting to edit the existing cases for broader use. Going to have to replace it with a new one which is extra fiddling.

[ColdAnkles]

That's something of a consequence of my choice to use the canonical name as the key for the map as well - the only way to do it would be to remove the existing named footprint and replace with an almost identical footprint.

[bubblobill]

Yup, just extra fiddling.
GUI is nearly there (old pic).

I have it on your fork so I am wary of pushing it until it is working otherwise you will need to exclude it from your commits.

[bubblobill]

Footprint functions:

1. People are going to ask for "Isometric" so you should include it, even if it only returns square.
2. You use mod in your offset translator calculations. Unwise as there is always the potential for a negative number to screw things up. Probably better to use bitwise And. (I know it's copy pasta, but that doesn't mean the original can't be improved on).

[ColdAnkles]

Some Feedback:
If you delete the only footprint marked as a default, you start getting errors -> set the first footprint in the list to be default if no defaults exist?
There's something I don't understand happening with the naming of footprints and the appending of underscores to them and this causing other footprints to disappear.
Its not immediately clear how the cell selection works; I worked out it was Select A, then B sets cells on A->B path as footprint - maybe use blank button to swap between path select and single cell select?
Cancel button didn't work.

[bubblobill]

Some Feedback: If you delete the only footprint marked as a default, you start getting errors -> set the first footprint in the list to be default if no defaults exist?

It is already supposed to do that

There's something I don't understand happening with the naming of footprints and the appending of underscores

Supposed to ensure unique names. Been redone a few times and I got distracted so it's probably between two different implementations

Cancel button didn't work.

Not sure if it's connected yet, but escape works.

Functionality is supposed to be:

• Typing in the text field: Changes the name of the currently selected footprint
• Add: Duplicates the current footprint with an amended name
• Revert: Goes back to the original footprint if it exists. Not intended to work on new footprints but it might.
• Clicking: Long story
  Firstly. The editing panel has gone through many iterations, in it's present form it accepts three arguments, a grid type name (nullable), a footprint, and a cell set (also nullable)(overrides getOccupiedCells()).
• creates an instance of each type of grid
• creates a new zone and zone renderer
• switches between grids according to passed arguments
• paints cell highlights for each of the footprint's cell set
• paints a waypoint marker on the origin cell that everything is relative to
• creates a ZoneWalker

The clicking:

• mouselistener gets cell under mouse from the zone renderer, sends it to modifyFootprintCells
• any clicks on cell(0,0) are rejected
• any other cell is then added to or removed from the current cell set
• if removed, the zone walker is reset to the origin point only
• when added it is also added as a waypoint to the zone walker and the walker's cell path is added to the cell set. This started as an attempt to ensure that there are no isolated cells but it doesn't work. Basically I'm tinkering with ways to make sure we end up with a monolithic geometry with no holes.

[bubblobill]

Surprised you didn't notice the abusable scale setting

[bubblobill]

Since there were no earth-shattering changes required in the Discord discussion, and nobody saw fit to dispute my unilateral decision making, I think we should be pushing this forward.

Copy of those unilateral decisions

Resurrecting this because I really want the editable footprints.

Firstly, disambiguation:

• Token anchor, scaleSize, scaleX, scaleY, and imageRotation relate purely to the token image being rendered.
• They can be applied to anything that is not free or native sized.
• Their relationship to the footprint is:
  • The anchor is relative to the footprint centre.
  • With everything at scale 1.0, the image fills the footprint.
• Token facing rotates the footprint itself.

Secondly, Properties of a Footprint

Origin Cell:

• Footprints have an origin cell from which all other cells radiate.
• The origin cell is defined as having its centre at 0,0
• It follows that sub-cell-sized footprints shrink uniformly within the cell, equidistant from the edges.
• For purposes of snap-to-grid this is the snapping point and it snaps to the cell centre.
  Inherent Points:
• Snap point, as above.
• Bounds centre, the centre of the rectangular bounds.
• Geometric centre, usually the mean of all vertices (centroid). The centre of the area defined by the footprint.
  Optional Points: probably a Map of point Sets.
• Centre of rotation.
• Perception points, e.g. normal vision.
• Emanation points, e.g. normal light.

Defaults

• Any unset point uses the bounds-centre. This mirrors the current way of things.

Optional Points

• Can exist on a footprint and/or a token allowing a token to override the return value.

Unilateral Decision

No more integers - I am tired of rounding errors screwing things up.

[ColdAnkles]

@bubblobill - I agree with basically all you've put forth, but am unsure whether any are worth pursuing immediately under this PR, or whether they should be postponed as a later enhancement
Also unsure on my own availability and ability to implement

[bubblobill]

More a roadmap for the future and design guide.
When dealing with graphics objects that are subject to offsets and rotation it is far easier to manipulate them if their reference point is their central point.
I keep wishing the functions were available in the work I'm doing, so that's an immediate benefit for me, and simply being able to define footprints is an immediate benefit to GMs everywhere.
What you have currently is perfectly fine. I'm mostly paving the way for what comes next.