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

adding resistance_anim to shema validation #4617

Merged
merged 7 commits into from Apr 20, 2020

Conversation

newfrenchy83
Copy link
Contributor

If a day, a resistance abilities applied to adjacent unit is added in the core, with a proper animation, it can be validated.

If a day, a resistance abilities applied to adjacent unit is added in the core, with a proper animation, it can be validated.
@CelticMinstrel
Copy link
Member

Um, excuse me? What are you doing renaming an existing animation without any sort of backwards compatibility? If it was just the first commit I would've merged it immediately, but the second two commits are all sorts of questionable.

@newfrenchy83
Copy link
Contributor Author

@CelticMinstrel after the first commit the travis checking failing because apply_to=(blade,pierce,tec) key became invalid. If necessary what animation name different what name of ability who call this animation for what shema validation valid.

@CelticMinstrel
Copy link
Member

That doesn't even make sense...

@newfrenchy83
Copy link
Contributor Author

@CelticMinstrel look at this https://travis-ci.com/wesnoth/wesnoth/jobs/262322472#L5140
"20191202 19:43:27 error validation: Invalid key 'impact=' in tag [resistance]" after the first commit, after that, i had the choice to close this PR or change the name of animation

@CelticMinstrel
Copy link
Member

Ah, I see the issue. The existing support for the resistance animation is [animation]apply_to=resistance, right? And you want to be able to shorten it to [resist_anim]. But… was the engine already trying to load the [resistance] tag as an animation? That seems like quite a large oversight…

@newfrenchy83
Copy link
Contributor Author

the engine

Ah, I see the issue. The existing support for the resistance animation is [animation]apply_to=resistance, right? And you want to be able to shorten it to [resist_anim]. But… was the engine already trying to load the [resistance] tag as an animation? That seems like quite a large oversight…

Like leading_anim for [leadership] [resistance] tags for animation already exist in engine for resistance ability, but has never used in core, or elsewhere in my knowledge.

@Wedge009 Wedge009 added the Schema label Dec 4, 2019
@newfrenchy83 newfrenchy83 changed the title add resistance anim to shema validation rename resistance animation and adding to shema validation Dec 16, 2019
@newfrenchy83 newfrenchy83 changed the title rename resistance animation and adding to shema validation adding resistance_anim to shema validation Jan 16, 2020
@newfrenchy83
Copy link
Contributor Author

@CelticMinstrel i have find the problem. I believe what "resistance" in udisplay was the namz complete of animation rhen what this is "resistance_anim", i corrected rhat now.

@gfgtdf
Copy link
Contributor

gfgtdf commented Apr 5, 2020

i'm neigher an expert on the animation wml codes, not on the shema, so i have no opinion on this.

@newfrenchy83
Copy link
Contributor Author

The problem is what it exist an reason for what nobody using [resistance_anim], if resistance ability can be used on offense or defense only, the animation will be always played in both case, if it is possible now to filter the type of damage applied with filter_weapon for filter the call of animation, it is not the case for active_on=defense/offenses or filter_base_value.

@CelticMinstrel
Copy link
Member

As a schema update, this looks fine. Does the[resistance_anim] tag actually get loaded as an animation in the game?

@newfrenchy83
Copy link
Contributor Author

Yes, when I use the resistance ability instead of leadership in an Elvish Captain, and replace leading_anim tags with resistance_anim tags, the animation is played when the assisted unit plays its [defense_anim] instead of its [attack_anim]

@gfgtdf gfgtdf removed their request for review April 6, 2020 13:51
@CelticMinstrel
Copy link
Member

In that case, yes, this should be (squash-)merged.

@Pentarctagon Pentarctagon merged commit 3a62c92 into wesnoth:master Apr 20, 2020
@newfrenchy83 newfrenchy83 deleted the patch-15 branch March 15, 2021 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants