Separate auras from lights #5357
Merged
Conversation
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
github-actions
bot
added
feature
|
No. You did not just make changes to the Token Properties dialog. |
Notice how I left this in draft 😄 I don't want to step on your work. |
Last time you did it was a nightmare. Another thought, The context menu is getting seriously oversized. This is it on my notebook monitor |
|
#5360 first to separate the layout tab, then you are free |
kwvanderlinde
force-pushed
the
feature/4598-separate-auras-from-lights
branch
from
016326c to
de297b7
Compare
|
I made #5374 for cleaning up the token menu. |
kwvanderlinde
force-pushed
the
feature/4598-separate-auras-from-lights
branch
2 times, most recently
from
12548ac to
409099b
Compare
The full set of lights is split into two separate collections on-demand: one for lights and one for auras. The lights are displayed in the Light tab, and the auras are displayed in the Auras tab. On commit, both are merged back together and committed to the campaign. `AuraSyntax` is also separated out from `LightSyntax`, but is a carbon copy for now. Neither has been reduced to reject inapplicable syntax yet. For some cleanup, the sightPanel, lightPanel, auraPanel, sightHelp, lightHelp, and auraHelp components are bound to fields in `CampaignPropertiesDialogView` so they can more conveniently be referenced without requiring `AbeillePanel#getComponent()`.
There is now a second text field for inputting auras as opposed to lights. Similar to the Campaign Properties dialog, a token's unique light sources will be split prior to display, and recombined upon commit.
With both unique light sources and unique auras, the panel is a bit bigger and not everything was set up well on the grid in order to stretch. The text fields for unique light sources and unique auras are once again inside a `JScrollPane` with a larger preferred height so that edit is less painful. Also removes empty rows and the pack of hspacers that was at the top of the config panel.
Light syntax is still represented by `LightSyntax`, while aura syntax is represented by `AuraSyntax`. Compared to previously, aura syntax does not support lumens. Internally, the `Light` objects for auras will be set to the default lumens value of 100. Note that with the current syntax, we cant *forbid* a lumens string from being provided, because our regex is not properly anchored. This is also true of lights: any random characters can be appended to the end of a range argument and they will simply be discarded without error or warning. Since this is not a new problem, I've left it as is, but I hope it can be tightened up in the future so there is no confusion. Lights no longer suppor the aura keyword, or the GM or OWNER arguments. Providing any of these will result in an error since they are not recognized and will be interpreted as a range argument.
Since auras are separated from lights everywhere else, it no longer makes sense to combine them in the token popup menu. Instead, auras have their own sub-menu and are no longer available in the lights sub-menu. This also removes the need for the "Clear All" action as we don't care to control lights and auras in concert. The existing "Clear Lights Only" and "Clear Auras Only" serve that purpose for the lights and auras submenus, respectively. These "Clear Lights Only" and "Clear Auras Only" have been renamed to just "Clear Lights" and "Clear Auras" since the context of their separate submenus already clarifies that. *This change has only been made in english translations and non-english translations that use the english defaults.*
The big deal here is that no one is allowed to implicitly modify the campaign's light sources map. I.e., we no longer return a mutable reference to the internal map. Instead, a copy is returned (unmodifiable map wouldn't be as nice since we have two levels of maps to deal with). Changes must be explicitly written back using `Campaign#setLightSourcesMap()` or `CampaignProperties#setLightSourcesMap()`. The next change is a nice-to-have and requires the former change to properly achieve. The outer map of the light sources map remains a `TreeMap<>`, but this is now notably enforced now via `readResolve()`. But the inner map has been changed from a `HashMap<>` to a `LinkedHashMap<>` to preserve insertion order. The `CampaignPropertiesDialog`, `LightSyntax`, and `AuraSyntax` also use `LinkedHashMap<>` now so that the user input is always preserved. This is a bit of a stop-gap solution while we still depend on the current structure of the light sources map. In the future I expect to replace this structure with something that does not depend on the same collection implementation being selected in a dozen different places.
This involves: 1. Copying help text from lights to auras. 2. Removing the `aura` keyword from lights and auras. 3. Removing any reference to lumens from auras. 4. Removing any reference to GM and OWNER keywords in lights. 5. Removing aura examples from lights and lights examples from auras. This required some changes to the translation keys used in the help text. Where possible, the changes have been reflected to all translation files. The legacy translation keys have also been updated, for the main `i18n.properties` file and any english translations.
This is largely a reformatting change. Key changes: - The wiki link at the top is now clickable and will open the page in the user's browser. - Show literal keywords in `<code>` tags, and do not provide a translated names for them. Applies to `scale`, `ignores-vbl`, `GM`, `OWNER`, `distance`, `arc`, `offset`, and `width`. - Show example values and default values in `<code>` tags. - Clarify which parts of sights, lights, and auras can be included several times. - Use `<pre>` tag for blocks of code instead `<code>` with embedded `<br>`. Also reduce the font size in the sections as they were rivaling the headings. - Only include actual syntax in example code blocks (i.e., no list numbering). Correspondingly, use a definition list for explaining examples where numbering was previous used. - Use `<h1>`/`<h2>` tags instead of custom formatted text for headings. - Allow the help text to scroll horizontally if needed, and increase the width of the panel slightly so it won't typically need to scroll. Also increase the height a bit for some more comfortable reading. A few corrections were also made: - Use the term "personal lights" instead of "personal sights" as the former has always been the term of choice. This also includes changing the translation key from `personalSight` to `personalLight`. English translations of the new key have been set to "Personal Light" while other languages have simply had the key removed. - Add translation keys for the `ignores-vbl` option. - Do not translate the various shape keywords. Some minor additions were made as well: - Added a "Name" row to the options table. - Added explanations for light examples.
The change to `LinkedHashMap<>` should not cause any issues, even with serialization, since we copy the map contents into our chosen implementation anyways. However, it does mean that new campaigns files with have `<linked-hash-map>` as the values under `<lightSourcesMap>`, where they used to have `<map>`. So to avoid any possible issues, and to meet the goal of zero campaign file changes in 1.18, we're going back to consistent `HashMap<>` for light source maps. The ubiquitous copying of light source maps remains.
kwvanderlinde
force-pushed
the
feature/4598-separate-auras-from-lights
branch
from
409099b to
f5a838d
Compare
|
Ready for review now. |
cwisniew
approved these changes
Apr 22, 2025
cwisniew
approved these changes
Apr 22, 2025
cwisniew
approved these changes
Apr 22, 2025
cwisniew
approved these changes
Apr 22, 2025
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
Closes #4598
Description of the Change
This is a UI change to configure and display auras separately from lights in the few places the two overlap:
As a UI-only change, this does not separate auras in the serialized campaign format, nor are existing macro functions affected. Instead, auras and lights are separated on demand when populated the above UI elements, and merged back together when committing new configurations. Because light sources are keyed by a hidden
GUIDrather than name, this means it is now possible to have an aura with the same name as a light. This is fine in terms of UI functionality, but can make the light functions a bit unwieldy since their name arguments can match both a light and an aura. For this reason, it isn't recommended to name auras the same as lights until an improved lighting API is provided.On the topic of macro functions, since we aren't changing the model, existing macros continue to work the same. The light macro functions can still detect, toggle, and clear auras as well as lights, just as they have always done.
With lights and auras being configurable separately, there is no longer a unified syntax between them. Instead they sport simiilar-but-distinct syntaxes that restrict configurations to their needs:
GMandOWNERflagsaurakeyword anymore.The syntax changes required updates to the help text so that it is clear what is accepted now and what is not, and what the differences are between lights and auras. While working on that, I found several peculiarities that I also changed:
scale,GM,OWNER,distance,arc,offset, andwidth, as well asignores-vblwhich was missing a translation key.circle,square, etc), but we do stiil allow a translatable explanatory string.<pre>tags rather than<code>with embedded<br>, and reduce the font size to normal so they aren't rivaling headings in size.<h1>/<h2>tags rather than custom enlarged text.There is actually one small change to the model, but it won't affect saved campaign formats or any other functionality. The
CampaignProperties.lightSourcesMapis no longer returned directly to any callers, and so cannot be modified directly as has been done for the Campaign Properties dialog. Instead, we now consistently do a deep copy of this map when passing it around. This is done inreadResolve()(to make sure we know what kind ofMap<>is used), in the copy constructor, and ingetLightSourcesMap(). A newsetLightSourcesMap()method allows callers to write back any changes that are needed.Possible Drawbacks
Existing lighting examples are now invalid syntax.
Documentation Notes
This is what (a portion of) the light help text used to look like:
It now looks like:
And for auras:
In the Edit Token dialog, we now have a separate box for unique auras:
The token popup menu has a separate submenu just for auras:
Release Notes
This change is