Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support switch-case replacements and definitions at root level #1459

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

TTtie
Copy link
Contributor

@TTtie TTtie commented Dec 15, 2024

Adds support for switch-case style replacement in message actions, inspired by pattern matching found in modern programming languages, for example (preliminary syntax, some details may be subject to change):

<message text="The outcome is {outcome}">
    <replacements>
        <!-- 
             value supports formulas and may be omitted 
             (in this case, the match attribute in <case>
             is disallowed)
        -->
        <switch id="outcome" value="variable">
            <!-- matches if the formula result is 0 -->
            <case match="0" result="Outcome 1"/>
            
            <!-- matches if the formula result is 1 AND only-team-1 allows -->
            <case match="1" filter="only-team-1" result="`aOutcome 2"/>
            
            <!-- matches if only-team-2 allows regardless of formula result -->
            <case filter="only-team-2" result="`bOutcome 3"/>
            
            <!--
                full syntax (matches if the formula result is between 2 and 5,
                and the inner filter allows)
            -->
            <case>
                <match>2..5</match>
                <filter>
                    <all id="not-moving">
                        <variable var="player.vel_x">0</variable>
                        <variable var="player.vel_y">..0</variable>
                        <variable var="player.vel_z">0</variable>
                    </all>
                </filter>
                <result>`cOutcome 4</result>
            </case>
            
            <!-- a fallback value if no cases match (defaults to an empty component) -->
            <fallback>`dA fallback outcome</fallback>
        </switch>
    </replacements>
</message>

Additionally, replacements can now be specified at the root level:

<map>
    <replacements>
        <!--
            scope is required for all replacements except <player />
            and it will be enforced if specified
        -->
        <decimal id="decimal" value="money" scope="player" format="0.00"/>
        <switch id="team" scope="team">
            <case filter="only-team-1" result="`aLime"/>
        </switch>
        
        <!-- This will error as only-team-1 is team-scoped -->
        <switch id="team2" scope="match">
            <case filter="only-team-1" result="`aLime"/>
        </switch>
    </replacements>
    <actions>
        <action id="action" scope="match">
            <switch-scope inner="player">
                <message text="The variable is {variable} and your team is {team}!">
                    <replacements>
                        <!--
                            the syntax for replacement references is
                            <replacement id="local-id">global-id</replacement>
                        -->
                        <replacement id="variable">decimal</replacement>
                        <replacement id="team">team</replacement>
                    </replacements>
                </message>
            </switch-scope>
        </action>
    </actions>
</map>

@TTtie TTtie force-pushed the feat/switch-case-replacements branch 6 times, most recently from e8cd0a1 to 6135522 Compare December 16, 2024 00:13
* @param audience The audience to use when replacing
* @return The replacement component
*/
ComponentLike replace(S audience);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not replacing you're creating a component, so i feel calling this method replace is a bit misleading
Maybe create or get would fit better

*
* @return The scope of the replacement
*/
Class<S> getScope();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can likely get away with not having a scope, but rather a checking function, since some replacements dont need any specific scope.

Think of it in the same spirit as respondsTo in filters.

Reason behind this is that if a replacement works for Match it should also work for party or player, there's no reason it shouldn't, at least not with the current replacements. If in the future a new replacement requires something else you can still implement that replacement as more strict, but for the ones we have it should be more lenient

throws InvalidXMLException {
scope = parseScope(el, scope);
Formula<T> formula = parser.formula(scope, el, "value").required();
var formatString = parser.string(el, "format").attr().orNull();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.optional().map(DecimalFormat::new).orElse(DEFAULT_FORMAT);

var nameStyle = parser.parseEnum(NameStyle.class, el, "style").optional(NameStyle.VERBOSE);
// Double cast is required, otherwise the compiler complains about type incompatibility
@SuppressWarnings("unchecked,RedundantCast")
Class<T> _scope = scope == null ? (Class<T>) (Object) Filterable.class : scope;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're doing the most scope-tricks for the replacement that needs them the least. The player replacement can literally work with any scope, it shouldnt even need to do any figuring-out


var result = parser.formattedText(innerEl, "result").required();

if (filter != null) {
Copy link
Member

@Pablete1234 Pablete1234 Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the place to write this code, make this part of the filter parser so you can parser.filter(el, "filter").respondsTo(scope).orNull()

branches.add(new SwitchReplacement.Branch(
result,
valueRange == null ? Range.all() : valueRange,
filter == null ? StaticFilter.ALLOW : filter));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use orAllow() at the end of the filter parser to avoid the nullcheck

*
* @param <S> The scope of the replacement
*/
public interface Replacement<S extends Audience> extends FeatureDefinition {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scopes should be extends Filterable<?>

Comment on lines 182 to 161
private void validateScope(Replacement<?> definition, Class<?> scope, Element el)
throws InvalidXMLException {
Class<?> definitionScope = definition.getScope();
if (!definitionScope.isAssignableFrom(scope))
throw new InvalidXMLException(
"Wrong replacement scope, got "
+ definitionScope.getSimpleName()
+ " but expected "
+ scope.getSimpleName(),
el);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned before, relegate this to a method within the replacement; let it decide what the logic for rejecting a scope should be

@TTtie TTtie force-pushed the feat/switch-case-replacements branch from 6135522 to 64dc8f7 Compare December 19, 2024 21:52
@TTtie TTtie requested a review from Pablete1234 December 19, 2024 21:55
@TTtie TTtie force-pushed the feat/switch-case-replacements branch 6 times, most recently from d2c9388 to 134a7b3 Compare December 22, 2024 11:46
Pablete1234
Pablete1234 previously approved these changes Dec 22, 2024
@TTtie TTtie marked this pull request as ready for review December 22, 2024 15:45
@TTtie TTtie requested a review from cswhite2000 as a code owner December 22, 2024 15:45
@TTtie TTtie force-pushed the feat/switch-case-replacements branch from 134a7b3 to b9c28c2 Compare December 22, 2024 15:53
@TTtie TTtie marked this pull request as draft December 22, 2024 15:53
@TTtie TTtie force-pushed the feat/switch-case-replacements branch 2 times, most recently from a8c582a to 7ed2058 Compare December 29, 2024 12:55
@TTtie TTtie force-pushed the feat/switch-case-replacements branch from 7ed2058 to ac17739 Compare January 4, 2025 14:24
@TTtie TTtie force-pushed the feat/switch-case-replacements branch 2 times, most recently from e97f48a to eeb41ed Compare January 17, 2025 22:43
@TTtie TTtie force-pushed the feat/switch-case-replacements branch from eeb41ed to c62a5c5 Compare February 2, 2025 13:39
TTtie added 2 commits February 8, 2025 17:29
Adds support for switch-case style replacement in message actions, inspired by pattern matching found in modern programming languages, for example

```xml
<action filter="game=1"><message text="`3Up Next: `b[1] Hungry Reindeer"/></action>
<action filter="game=2"><message text="`3Up Next: `b[2] Present Hunt"/></action>
<action filter="game=3"><message text="`3Up Next: `b[3] Winter Speedway"/></action>
<action filter="game=4"><message text="`3Up Next: `b[4] Parkour"/></action>
<action filter="game=5"><message text="`3Up Next: `b[5] Chimney Sweepers"/></action>
<action filter="game=6"><message text="`3Up Next: `b[6] Frosted Corridors"/></action>
```
becomes
```xml
<message text="`3Up Next: `b[{game}] {game-name}">
    <replacements>
        <decimal id="game" value="game"/>
        <switch id="game-name" value="game">
            <case value="1" result="Hungry Reindeer"/>
            <case value="2" result="Present Hunt"/>
            <case value="3" result="Winter Speedway"/>
            <case value="4" result="Parkour"/>
            <case value="5" result="Chimney Sweepers"/>
            <case value="6" result="Frosted Corridors"/>
        </switch>
    </replacements>
</message>
```

It is also possible to use them with filters:
```xml
<message text="The bomb has been planted on {bombsite} by {holder}">
    <replacements>
        <player id="holder" var="bomb_holder" fallback="an unknown player"/>
        <switch id="bombsite">
            <case filter="planted-a" result="`c${bombsite-a-name}"/>
            <case filter="planted-b" result="`c${bombsite-b-name}"/>
        </switch>
    </replacements>
</message>
```

and also to mix them together:
```xml
<message text="Your score is {score} ({score_num})!">
    <replacements>
        <decimal id="score_num" value="score"/>
        <switch id="score" value="score">
            <case value="0..70" filter="only-attackers" result="`cvery low :("/>
            <case value="0..70" filter="only-defenders" result="`9very low :(" />
            <case value="70..100" filter="only-attackers" result="`clow :|"/>
            <case value="70..100" filter="only-defenders" result="`9low :|" />
            <case value="100..130" filter="only-attackers" result="`chigh :)"/>
            <case value="100..130" filter="only-defenders" result="`9high :)" />
            <fallback>off the charts :O</fallback>
        </switch>
    </replacements>
</message>
```

Signed-off-by: TTtie <[email protected]>
Allows for global replacements to be registered on the map level:
```xml
<replacements>
    <switch id="bombsite">
        <case filter="filter-a" result="filter-a-result"/>
        <case filter="filter-b" result="filter-b-result"/>
    </switch>
</replacements>
```

Where required, a global replacement requires a matching scope (in actions, the scope is inherited from the current scope):
```xml
<replacements>
    <switch value="variable" scope="match">
        ...
        <fallback>...</fallback>
    </switch>
</replacements>
```

A globally defined replacement then can be referred to from within as:
```xml
<!-- somewhere in <map> -->
<replacements>
    <player id="global-replacement" var="variable" fallback="an unknown player"/>
</replacements>

<!-- in an action -->
<message text="... {replacement}">
    <replacements>
        <replacement id="replacement">global-replacement</replacement>
    </replacements>
</message>
```

To support for this, replacement parsing has been forked out of ActionParser.

Signed-off-by: TTtie <[email protected]>
@TTtie TTtie force-pushed the feat/switch-case-replacements branch from c62a5c5 to eeebf4e Compare February 8, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants