Fix for missing state image IDs in campaign files#6008
Draft
kwvanderlinde wants to merge 8 commits into
Draft
Conversation
0fd85c9 to
5d92145
Compare
This makes campaign writing more robust as it can simply ask for the list of relevant assets rather than first trying to figure out the types of all the states and bars.
`AbstractImageTokenOverlay` is the base for `ImageTokenOverlay`, `CornerImageTokenOverlay`, and `FlowImageTokenOverlay`. In places where a type check is needed to look for an image-based overlay, `AbstractImageTokenOverlay` is now checked instead of `ImageTokenOverlay`. The new base class also provides an implementation of `#getAssetId()` and `#getAssets()` so child classes don't have to worry about it.
`Campaign#getAllAssetIds()` new leverages `CampaignProperties#getAllImageAssets()` rather than duplicating the same logic for lookup tables, bars, and states. `CampaignProperties#getAllImageAssets()` in turn now uses `AbstractTokenOverlay#getAssets()` rather than enumerating the possible types that could have assets.
- `getBarImage()` macro function implementation and `TokenBarController` to use the new list-based `AbstractTokenOverlay#getAssets()`
- `TokenStatesController` uses switch for exhaustiveness over all state types
- `getInfo("campaign")` shows the grid size for shape-based flow states and image-based flow states
- `getStateImage()` macro function checks for `AbstractImageTokenOverlay` rather than `ImageTokenOverlay``
…setIds() In `MultipleImageBarTokenOverlay`, replace the array-based accessor and mutator with `List`-based ones.
5d92145 to
fb8dedc
Compare
Each implementation now sets all relevant properties (a bunch of these were missed after splitting out `XTokenOverlay` as a base class). Where possible, the logic is shared according to base class rather than replicating it per concrete class.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Identify the Bug or Feature request
Fixes #6004
Fixes #5933, introduced in PR #5829
Description of the Change
First of all, updates the various places that states and bars are added to a campaign so that they also upload the asset to the server if needed. The places are the states and bar controllers of the Campaign Properties dialog, and the
addtokenstateandloadtokenstatesmacro definitions. This fixes #6004Supporting that, and fixing #5933, all token overlays (states and bars) have a general
#getAssetIds()method that returns the IDs of all assets required by the overlay. This replaces most previous checks forImageTokenOverlayas that was no longer the base class for all image-bearing states since #5829. This is especially important when serializing the campaign, as all state and bar assets are now guaranteed to be included.There are a few places where checking for image-bearing states is still useful, particular some macro functions and UI code. A new
AbstractImageTokenOverlayclass has been introduced to fill that role without mixing up the individual concrete classes.Misc notes:
getInfo("campaign"), there is still a need to check concrete types to provide the"corner"and"gridSize"keys for associated states. As these are specific to this macro function, I haven't added any kind of generic way of expressing these.TokenStatesController, an exhaustive switch is now used to make sure we handle all the possible cases.Possible Drawbacks
None
Documentation Notes
N/A
Release Notes
N/A
This change is