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

Invalid [damage_type] [filter_opponent] formula causes silent crash #8522

Open
Dalas121 opened this issue Mar 9, 2024 · 11 comments
Open

Invalid [damage_type] [filter_opponent] formula causes silent crash #8522

Dalas121 opened this issue Mar 9, 2024 · 11 comments
Labels
Bug Issues involving unexpected behavior. WFL Issues involving the Wesnoth Formula Language engine and APIs.

Comments

@Dalas121
Copy link
Contributor

Dalas121 commented Mar 9, 2024

Game and System Information

  • Version: 1.17.26
  • Downloaded from: Steam
  • OS: Windows

Description of the bug

Targeting a weapon attack with the following invalid specials causes Wesnoth to crash without an error.

The formula has an extra closing parenthesis, which causes the crash. Crashing is fine, error-less is the bug.

            [damage_type]
                [filter_opponent]
                    formula="$( $self[arcane] >= max([ $self[blade], $self[impact], $self[pierce], $self[arcane], $self[cold], $self[fire] ]) ))"
                [/filter_opponent]
                alternative_type=arcane
            [/damage_type]
            [damage_type]
                [filter_opponent]
                    formula="$( $self[fire] >= max([ $self[blade], $self[impact], $self[pierce], $self[arcane], $self[cold], $self[fire] ]) ))"
                [/filter_opponent]
                alternative_type=fire
            [/damage_type]
            [damage_type]
                [filter_opponent]
                    formula="$( $self[cold] >= max([ $self[blade], $self[impact], $self[pierce], $self[arcane], $self[cold], $self[fire] ]) ))"
                [/filter_opponent]
                alternative_type=cold
            [/damage_type]

Steps to reproduce the behavior

  1. Assign the listed specials to a unit's weapon
  2. Try to make an attack with the unit

Expected behavior

Wesnoth should throw an error, recognizing the invalid formula syntax.

Additional context

No response

@Dalas121 Dalas121 added the Bug Issues involving unexpected behavior. label Mar 9, 2024
@Dalas121 Dalas121 changed the title Invalid "formula" key causes silent crash Invalid [damage_type] [filter_opponent] formula causes silent crash Mar 9, 2024
@cooljeanius
Copy link
Contributor

semi-related: #8385

@newfrenchy83
Copy link
Contributor

The alternative_type choose automaticelly the damage type who done more damage but for the weapon applied, here i see what you use formula self[cold] >max[cold] perhaps what you should remove max damage when same type what damage compared

@newfrenchy83
Copy link
Contributor

Oh pardon, i dont undertstand what bug is intentional but not behavior of game when error in formula, have you trier with other weapn special ?

@newfrenchy83
Copy link
Contributor

Like alternative_type select the damage type who done more damage between original and [damage_type]alternative_type= why you encode this in [filter_oppoonent]?

@newfrenchy83
Copy link
Contributor

newfrenchy83 commented Mar 9, 2024

I have tried several times and the problem is not specific to [damage_type], I don't know why wesnoth crashes without displaying a syntax error message in log files and I don't see how to fix it

@CelticMinstrel
Copy link
Member

Crashing is fine

No. No it's not. Crashing is never fine, not even if you've coded something incredibly dumb (which yes, this is). There should always be an error message instead of a crash.

[damage_type]
    [filter_opponent]
        formula="$( $self[arcane] >= max([ $self[blade], $self[impact], $self[pierce], $self[arcane], $self[cold], $self[fire] ]) ))"
    [/filter_opponent]
    alternative_type=arcane
[/damage_type]
[damage_type]
    [filter_opponent]
        formula="$( $self[fire] >= max([ $self[blade], $self[impact], $self[pierce], $self[arcane], $self[cold], $self[fire] ]) ))"
    [/filter_opponent]
    alternative_type=fire
[/damage_type]
[damage_type]
    [filter_opponent]
        formula="$( $self[cold] >= max([ $self[blade], $self[impact], $self[pierce], $self[arcane], $self[cold], $self[fire] ]) ))"
    [/filter_opponent]
    alternative_type=cold
[/damage_type]

Uhhhhhhh… what on earth are you doing here? The extra closing parenthesis is just the start of the errors in that snippet.

  1. Extra closing parenthesis bad, obviously
  2. $self[arcane] is not a valid WML variable substitution (though the variable parser is rather forgiving, so I guess there's a chance that it parses it equivalent to self.arcane – something which in my opinion shouldn't be supported, mainly just to keep things simple)
  3. You're using a formula substitution ($()) in formula= - there is no reason to ever do this

The correct formula would be something like this (assuming no custom damage types are in play):

formula="resistance.fire >= max(values(resistance))"

Though it can never be greater, so you might as well delete the > as well. If custom damage types are in play and you only want to check the default ones, you can write out all the resistance.whatever inside the max() in place of values(resistance).

Though obviously all of that is assuming that you want "self" to refer to the opponent and not some random other unit on the map. Still, if you do want "self" to refer to some random other unit on the map, and and $self is a stored unit, I think all you need to do is append where self = unit_at($self.x, $self.y) to the formula.

@Dalas121
Copy link
Contributor Author

Dalas121 commented Mar 9, 2024

Yeah, the snippet is very wrong. Right now I've got it all working correctly, I appreciate the notes.

#define ALTERNATIVE_DAMAGE_TYPE TYPE
    [damage_type]
        [filter_opponent]
            formula=" self.resistance['{TYPE}'] = min([ self.resistance['blade'], self.resistance['impact'], self.resistance['pierce'], self.resistance['arcane'], self.resistance['cold'], self.resistance['fire'] ]) "
        [/filter_opponent]
        alternative_type={TYPE}
    [/damage_type]
#enddef
[dummy]
    name=_"astral"
    description=_"This attack always deals whichever damage type its opponent is most vulnerable to."
[/dummy]
{ALTERNATIVE_DAMAGE_TYPE cold}
{ALTERNATIVE_DAMAGE_TYPE fire}
{ALTERNATIVE_DAMAGE_TYPE arcane}
{ALTERNATIVE_DAMAGE_TYPE blade}
{ALTERNATIVE_DAMAGE_TYPE pierce}
{ALTERNATIVE_DAMAGE_TYPE impact}

@CelticMinstrel
Copy link
Member

I think self.resistance.blade should also work – from what I remember, WFL allows you to address maps "as if" they were an object as long as the keys are strings that are valid identifiers.

And as mentioned in the previous comment, the values(self.resistance) thing would still simplify your formula if you don't mind it also catching custom damage types.

@Dalas121
Copy link
Contributor Author

Dalas121 commented Mar 9, 2024

Will do, thanks. Much simpler for sure, and I don't mind catching custom damage types.

@newfrenchy83
Copy link
Contributor

newfrenchy83 commented Mar 9, 2024

Yeah, the snippet is very wrong. Right now I've got it all working correctly, I appreciate the notes.

#define ALTERNATIVE_DAMAGE_TYPE TYPE
    [damage_type]
        [filter_opponent]
            formula=" self.resistance['{TYPE}'] = min([ self.resistance['blade'], self.resistance['impact'], self.resistance['pierce'], self.resistance['arcane'], self.resistance['cold'], self.resistance['fire'] ]) "
        [/filter_opponent]
        alternative_type={TYPE}
    [/damage_type]
#enddef
[dummy]
    name=_"astral"
    description=_"This attack always deals whichever damage type its opponent is most vulnerable to."
[/dummy]
{ALTERNATIVE_DAMAGE_TYPE cold}
{ALTERNATIVE_DAMAGE_TYPE fire}
{ALTERNATIVE_DAMAGE_TYPE arcane}
{ALTERNATIVE_DAMAGE_TYPE blade}
{ALTERNATIVE_DAMAGE_TYPE pierce}
{ALTERNATIVE_DAMAGE_TYPE impact}

I see that you use the alternative_type attribute in [damage_type] but do you know that if you use it instead of replacement_type then you do not need a formula because the resistance comparison is programmed in the source code and alternative_type chosen always the type for which the opponent is least resistant, but in case where you use many [damage_type] difffrent it is make sense.

@newfrenchy83
Copy link
Contributor

newfrenchy83 commented Mar 9, 2024

if you need to choice the alternative_type in list of many damage_type, then yes use formula for exclude of list used the damage_type who don't matches is a good method, the altenative_type used alone is valable for a only [damage_type] else this isthe [damage_type] in head of list who will be used(the type are classed in alphabétical order by default in damage_type)

@Wedge009 Wedge009 added the WFL Issues involving the Wesnoth Formula Language engine and APIs. label Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues involving unexpected behavior. WFL Issues involving the Wesnoth Formula Language engine and APIs.
Projects
None yet
Development

No branches or pull requests

5 participants