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

Insert [filter weapon] in filter student/opponent/etc.] for weapon like abilities(1.15/1.16) #4435

Conversation

@newfrenchy83
Copy link
Contributor

commented Oct 7, 2019

i doing stupid thing in old PR and a post another in replacement for resolve #4389

newfrenchy83 added 5 commits Oct 7, 2019
@newfrenchy83 newfrenchy83 changed the title Insert [filter weapon] in filter student/opponent/etc.] for weapon like abilities Insert [filter weapon] in filter student/opponent/etc.] for weapon like abilities(1.15/1.16) Oct 8, 2019
@newfrenchy83

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2019

@CelticMinstrel , @gfgtdf Forgive me for insisting but could someone look into this PR, please?

…ponent/etc.]
@jostephd

This comment has been minimized.

Copy link
Member

commented Oct 12, 2019

@CelticMinstrel @newfrenchy83 Is this a 1.16 blocker? Does it fix a regression, or just add a new feature?

@newfrenchy83

This comment has been minimized.

Copy link
Contributor Author

commented Oct 12, 2019

@jostephd Is adda new feature for what abilities like weapon was similar to true weaoon special in wml code

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Oct 14, 2019

@jostephd From my understanding it's a modification of syntax for greater consistency.

@newfrenchy83 Does this actually remove [filter_weapon] and [filter_second_weapon]? In case it does, we can't just remove them. They should produce a deprecation warning when used, but they should still continue to work. As long as that's the case, this can be merged.

@newfrenchy83

This comment has been minimized.

Copy link
Contributor Author

commented Oct 14, 2019

@CelticMinstrel i fear to let [filter_weapon] and [filter_second_weapon] because possibity of bugs if someone put in abilities weapon [filter_weapon] int [filter_student/opponent/etc) and filter_weapon/filter_second_weapon] simultaneous. and i don't see where put deprecation message in that case.

@newfrenchy83

This comment has been minimized.

Copy link
Contributor Author

commented Oct 14, 2019

@CelticMinstrel and @jostephd [filter_weapon/filter_second_weapon] not deprecated for [leadership] and [resistance] abilities, but must be removed of weapon like abilities now what syntax modifed

@newfrenchy83

This comment has been minimized.

Copy link
Contributor Author

commented Oct 14, 2019

an idea will be to using weapon for [leadership] and to erase [filter_second_weapon] and filter_weapon to [resistance]

…ponent/etc.]
@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Oct 15, 2019

If they use both [filter_(second_)weapon] and the nested [filter_weapon] that should replace it, then it's fair to make the new syntax take priority over the old - so just ignore the deprecated one (other than printing a deprecation message).

@newfrenchy83

This comment has been minimized.

Copy link
Contributor Author

commented Oct 15, 2019

If they use both [filter_(second_)weapon] and the nested [filter_weapon] that should replace it, then it's fair to make the new syntax take priority over the old - so just ignore the deprecated one (other than printing a deprecation message).

and how i doing that please?

@@ -1687,7 +1691,7 @@ std::pair<int, bool> ability_leadership(const std::string& ability,const unit_ma
} else if(up == units.end() && (filter_student || filter || filter_attacker || filter_defender)) {
return {abil_value, false};
} else {
show_result = !(!un->abilities_filter_matches(*i->first, attacker, abil_value) || ability_apply_filter(un, up, ability, *i->first, loc, opp_loc, attacker));
show_result = !(!un->abilities_filter_matches(*i->first, attacker, abil_value) || ability_apply_filter(un, up, ability, *i->first, loc, opp_loc, attacker, weapon, opp_weapon));

This comment has been minimized.

Copy link
@newfrenchy83

newfrenchy83 Oct 15, 2019

Author Contributor

ability_apply_filter in after un->get_abilities(ability, weapon, opp_weapon); [filter_weapon/filtr_second_weapon] will be used in all case or these filter will be in abilitie like weapon in same time what usual code syntax what i want emulate, impossible to create an priority to the last, the only solution is to erase [filter_weapon/second_weapon] directly. It is one or other but not both.

i know what i must using an deprecation method, but it is impossible here. for don't have bugs, it must erase  [filter(_second)_weapon] to weapon like abilities and using that what for [leadership] and [resistance]. Sorry but i don't find another idea.
i haven't not see what i can't use two filter different in same abilitie, and i must correct that
if [filter(second_weapon) not present of if[filter_student/opponent/etc..] presnet then weapon not filtered. Used for possibility of conflict in abilities weapon like.
…ponent/etc.]
@newfrenchy83

This comment has been minimized.

Copy link
Contributor Author

commented Oct 17, 2019

If they use both [filter_(second_)weapon] and the nested [filter_weapon] that should replace it, then it's fair to make the new syntax take priority over the old - so just ignore the deprecated one (other than printing a deprecation message).

@CelticMinstrel i create priority for new syntax, i new syntax is present, old willbe blocked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.