RPTools
Token Footprint Functions and Campaign Properties
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 
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
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
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. :)
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 :)
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.
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
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?
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
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
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.
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
Didn't someone recently fix something relating to footprints? Maybe in #4830?
It's definitely something related to my changes - develop and 1.14.3 don't have the issue
Swap your vertical and horizontal hex logic.
I'm not following (unless you meant to swap makeVertHex and makeHorizHex in which case I did, and had no effect.)
The hex that is out of place could have resulted from being calculated using the wrong grid type. Was hoping it was an easy fix due to a bad offset calc.
as best I can tell, for both grids, every second row of cells has off-by-one coordinates and I'm a little lost on where to go find the source of the issue
This is correct. I thought your problem may be the you are getting the offset on the wrong row/column. Try incrementing it by one and see if the shape ends up correct. You may simply be getting even offsets on odd rows and vice versa.
Go here and follow the link to redBlobGames. Read his stuff on hex grids.
https://github.com/RPTools/maptool/wiki/On-Maps,-Grids,-Objects,-and-Location
Assuming my understanding of hex grids is functional, here's a better diagram from what I'm observing:
What's up with
(-1,-1) under actual? That entire row is off by one.
May need to adjust your expectation! ;)
MapTool seems to actually use the following coordinates for pointy-top hex grids:
and for flat-top hex grids:
These align with the hex "odd-r" and "odd-q" offset coordinate systems mentioned here: https://www.redblobgames.com/grids/hexagons/#coordinates-offset
oh barf, I hate that Do we have any reason to keep it that way? (and thanks and kudos for the really clear and helpful diagram)
oh barf, I hate that Do we have any reason to keep it that way? (and thanks and kudos for the really clear and helpful diagram)
No worries about the diagram - I just expanded on your better diagram idea! 👍
Maybe a separate FR to introduce a whole new (optional) alternative to offset hex grid coordinate referencing, but it is possible to translate between offset and axial coordinates: https://www.redblobgames.com/grids/hexagons/#conversions-offset
The problem now is that footprints are defined as a number of offsets from a reference point - the standard "Large" size token is comprised of the cells (0,0), (1,0), (0,1) which works A-OK when the reference point is also (0,0) since adding offsets to that results in identical set. But when the reference point is (-1,0) you end up with the set (-1,0), (0,0), (-1,1) instead of (-1,0), (0,1), (-1,1) - two entirely different shapes.
I have no idea how it was working previously and why it's different now, since I'm using the same list of offsets as were originally used.
Is it worth telling the getOccupiedCells function what grid you're on and attempting to handle the offset there, or something else?
If you read the redblob stuff about coordinate systems you will see that there are much better systems than the odd-r/q crap we went with.
Since I have been advocating we upgrade to cubic ever since I read about it I think this is a great opportunity to implement it. You wouldn't need a new PR to change the hard-coded xml to fit a sensible system, and adding a third axis will make future stuff much easier. Calculating radii and rings and other shapes programatically is much easier with a decent coordinate system. He even provides code for conversion between systems so a tiny bit of infill code can keep things running that don't rely on it. You can do all your operations on sensible stuff and just spit out the ugly for every other function to use.
oh barf, I hate that
Do we have any reason to keep it that way?
(and thanks and kudos for the really clear and helpful diagram)
No reason. Currently every size is just a bigger circle. You could throw out the existing stuff and generate them on startup in a couple of ms.
I'm all for using a proper coordinate system (y-up) instead of the pixel-based (0,0) is top left. But this should be done everywhere and not just in a small part like tokenfootprint.