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: fear condition #542

Merged
merged 37 commits into from
May 20, 2023
Merged

feat: fear condition #542

merged 37 commits into from
May 20, 2023

Conversation

bosmak
Copy link
Contributor

@bosmak bosmak commented Oct 7, 2022

Description

Implement fear condition.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I checked the PR checks reports
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

@dudantas
Copy link
Member

dudantas commented Oct 7, 2022

Your pull request looks great, I approved the execution of the checks. I'll take a closer look later, but from what I've seen, everything seems ok.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 7, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell B 954 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@guispiller guispiller left a comment

Choose a reason for hiding this comment

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

Thank you, excellent PR!
Tested here, feared condition working fine restricting for party members, but still have some issues/restrictions for the condition itself, i'll point here:

  • Fear condition when activated, should remain for 6 seconds;
  • After being feared, when the condition finishes, you should have 10 seconds "cooldown" until you can get feared again;
  • When feared, you should not be able to use any spell, nor equip amulets/rings (at the pr sometimes you can use spell, and can equip items anytime)

@dudantas
Copy link
Member

dudantas commented Nov 9, 2022

Thank you, excellent PR! Tested here, feared condition working fine restricting for party members, but still have some issues/restrictions for the condition itself, i'll point here:

  • Fear condition when activated, should remain for 6 seconds;
  • After being feared, when the condition finishes, you should have 10 seconds "cooldown" until you can get feared again;
  • When feared, you should not be able to use any spell, nor equip amulets/rings (at the pr sometimes you can use spell, and can equip items anytime)

I'll add the pull as a draft until the author decides to analyse it and, if necessary, fix the problems.

@dudantas dudantas marked this pull request as draft November 9, 2022 03:24
@bosmak
Copy link
Contributor Author

bosmak commented Nov 9, 2022

Sorry for the delay, I have been stuck on some work-related topics.

I will try to work on it this week!

@bosmak
Copy link
Contributor Author

bosmak commented Nov 14, 2022

@dudantas
I tried to remove the XML function, but it seems more code is attached, so I left it as is.

@guispiller
I couldn't reproduce using a spell under the fear condition.
Besides that, I adjusted the default duration to 6 seconds, Fixed that it was possible to change the equipment using the hotkeys, and finally added the cooldown for 10 seconds after being feared.

Could you test again?


For other contributors looking to test, just add the following attack to a monster

{name ="fear", interval = 2000, chance = 100, range=7, effect=CONST_ME_MAGIC_RED, target=true}

@bosmak bosmak marked this pull request as ready for review November 14, 2022 01:31
…llCheck function. Akk recommended this function to ensure no spell can be casted.
@tutbarao
Copy link

Tested! Functional!

Copy link

@jlcvp jlcvp left a comment

Choose a reason for hiding this comment

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

Looking great, just a few suggestions

@bosmak bosmak requested review from guispiller and removed request for guispiller November 17, 2022 20:28
@paczielny
Copy link

Severity Code Description Project File Line Suppression State
Error C3668 'ConditionFeared::executeCondition': method with override specifier 'override' did not override any base class methods

when try build :/

@dudantas
Copy link
Member

Severity Code Description Project File Line Suppression State Error C3668 'ConditionFeared::executeCondition': method with override specifier 'override' did not override any base class methods

when try build :/

you must be using some mixture of prs or something, because the compilation checks are passing normally

@paczielny
Copy link

Severity Code Description Project File Line Suppression State Error C3668 'ConditionFeared::executeCondition': method with override specifier 'override' did not override any base class methods
when try build :/

you must be using some mixture of prs or something, because the compilation checks are passing normally

maybe version? 13.11

@mohuga
Copy link

mohuga commented Feb 27, 2023

character dont walk after feared, just stuck! but conditions of heal, time, etc still working correctly

@dudantas
Copy link
Member

Severity Code Description Project File Line Suppression State Error C3668 'ConditionFeared::executeCondition': method with override specifier 'override' did not override any base class methods
when try build :/

you must be using some mixture of prs or something, because the compilation checks are passing normally

maybe version? 13.11

You need to test on the pull request branch only. The error you have is not related to the pull request itself, but its implementation.

@elsongabriel
Copy link
Contributor

elsongabriel commented Feb 27, 2023

Severity Code Description Project File Line Suppression State Error C3668 'ConditionFeared::executeCondition': method with override specifier 'override' did not override any base class methods
when try build :/

you must be using some mixture of prs or something, because the compilation checks are passing normally

maybe version? 13.11

You need to test on the pull request branch only. The error you have is not related to the pull request itself, but its implementation.

I merged 3 branches.. this (fear_condition), test (client 13.11), and missing_bosses, it works!
we have to pay attention when merging with PR client 13, to use the functions with int64_t parameters.
@paczielny

@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2023

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added Stale No activity and removed Stale No activity labels Apr 2, 2023
@dudantas
Copy link
Member

Severity Code Description Project File Line Suppression State Error C3668 'ConditionFeared::executeCondition': method with override specifier 'override' did not override any base class methods
when try build :/

you must be using some mixture of prs or something, because the compilation checks are passing normally

maybe version? 13.11

You need to test on the pull request branch only. The error you have is not related to the pull request itself, but its implementation.

I merged 3 branches.. this (fear_condition), test (client 13.11), and missing_bosses, it works! we have to pay attention when merging with PR client 13, to use the functions with int64_t parameters. @paczielny

Do you consider that the system is ok to be merged? Now we no longer have the changes to int64_t, which should make it easier for the system to implement.

@paczielny
Copy link

how add fear condition in attack of monsters?

@paczielny
Copy link

how add fear condition in attack of monsters?

nevermind

@paczielny
Copy link

image

compiled in 13.16

@paczielny
Copy link

image

@dudantas
Copy link
Member

image

compiled in 13.16

Please, test again with my last commit. 11892d6

@paczielny
Copy link

image
compiled in 13.16

Please, test again with my last commit. 11892d6

working perfectly! thanks @dudantas

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 9 Code Smells

0.0% 0.0% Coverage
3.6% 3.6% Duplication

@dudantas dudantas merged commit 801c7bc into opentibiabr:main May 20, 2023
@bosmak bosmak deleted the fear_condition branch May 21, 2023 08:27
enzerah pushed a commit to enzerah/canary that referenced this pull request May 21, 2023
The Fear Condition is a status condition that makes the affected creature lose control of their movements for a short period of time. During this period, the affected creature randomly moves around in various directions, unable to perform any other actions. This condition is usually brought about by certain spells or monster effects.
enzerah pushed a commit to enzerah/canary that referenced this pull request May 21, 2023
The Fear Condition is a status condition that makes the affected creature lose control of their movements for a short period of time. During this period, the affected creature randomly moves around in various directions, unable to perform any other actions. This condition is usually brought about by certain spells or monster effects.
luan pushed a commit that referenced this pull request Jul 11, 2023
The Fear Condition is a status condition that makes the affected creature lose control of their movements for a short period of time. During this period, the affected creature randomly moves around in various directions, unable to perform any other actions. This condition is usually brought about by certain spells or monster effects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing Hex, Fear and Goshnar Taints conditions