Initial implementation of Soul Ascetic #8162

Lemongrass3110 · 2024-03-11T01:14:03Z

Addressed Issue(s): None
Server Mode: Renewal
Description of Pull Request:
Taken from 4th / Expanded Job Skill-Updates as of KRO 20220602 incl. 3rd Job fixes / Homunculus skills #7024

Taken from #7024

src/map/skill.cpp

Lemongrass3110 · 2024-03-11T01:53:44Z

src/map/skill.cpp

 		if (sd == NULL || sd->status.party_id == 0 || (flag & 1)) {

 			// Animations don't play when outside visible range
 			if (check_distance_bl(src, bl, AREA_SIZE))
 				clif_skill_nodamage(bl, bl, skill_id, skill_lv, 1);

+			if( skill_id == SOA_SOUL_OF_HEAVEN_AND_EARTH ){
+				if( src != bl && tsc && tsc->getSCE(SC_TOTEM_OF_TUTELARY) ){
+					status_heal(bl, 0, 0, 3 * skill_lv, 0);


This is the special part about Totem of Tutelary that heals AP:
Using this skill while the user is under the effect of Totem of Tutelary also recovers AP to the party members.

But where is the normal healing part:
Calls the divine power from the heaven upon the earth, fully recovers SP to all party members within 23 x 23 cells around the user.

For me this implementation looks wrong, as it checks the target sc instead of the source sc.

https://divine-pride.net/database/skill/5432

src/map/skill.cpp

Lemongrass3110 · 2024-03-11T02:09:51Z

src/map/skill.cpp

+					int hp = 500;
+
+					hp += 500 * sg->skill_lv;
+					hp += 50 * pc_checkskill( tsd, SOA_TALISMAN_MASTERY ) * sg->skill_lv;


Suggested change

hp += 50 * pc_checkskill( tsd, SOA_TALISMAN_MASTERY ) * sg->skill_lv;

hp += 50 * pc_checkskill( BL_CAST( BL_PC, ss ), SOA_TALISMAN_MASTERY ) * sg->skill_lv;

According to divine-pride it should depend on the users level of talisman mastery and not the targets level of talisman mastery:

Recovers additional amount depends on level of Talisman Mastery user learned, user's base level and CRT.

https://divine-pride.net/database/skill/5422

src/map/skill.cpp

@datawulf

Confirmed by @datawulf

@datawulf

Confirmed by @datawulf

Lemongrass3110 · 2024-03-11T10:05:54Z

db/re/skill_db.yml

+          Size: 3
+        - Level: 5
+          Size: 4
+      Interval: 1000


Suggested change

Interval: 1000

Interval: 500

According to @datawulf

Lemongrass3110 · 2024-03-11T10:16:03Z

src/map/skill.cpp

+
+					hp += 500 * sg->skill_lv;
+					hp += 50 * pc_checkskill( tsd, SOA_TALISMAN_MASTERY ) * sg->skill_lv;
+					hp += 50 * status_get_crt( ss ) * sg->skill_lv;


Suggested change

hp += 50 * status_get_crt( ss ) * sg->skill_lv;

hp += 5 * status_get_crt( ss );

According to @datawulf

Lemongrass3110 · 2024-03-11T10:16:53Z

src/map/skill.cpp

+
+					sp += 50 * sg->skill_lv;
+					sp += 5 * pc_checkskill( BL_CAST( BL_PC, ss ), SOA_TALISMAN_MASTERY ) * sg->skill_lv;
+					sp += 5 * tstatus->crt * sg->skill_lv;


Suggested change

sp += 5 * tstatus->crt * sg->skill_lv;

sp += 5 * tstatus->crt;

According to @datawulf

Lemongrass3110 · 2024-03-11T10:17:31Z

src/map/skill.cpp

+					sp *= 100 + status_get_lv( ss );
+					sp /= 150;


Suggested change

sp *= 100 + status_get_lv( ss );

sp /= 150;

sp *= status_get_lv( ss ) / 100;

According to @datawulf

… into class/soul_ascetic

# Conflicts: # db/re/status.yml

Lemongrass3110 · 2024-03-11T23:18:24Z

src/map/skill.cpp

 		if (sd == NULL || sd->status.party_id == 0 || (flag & 1)) {

 			// Animations don't play when outside visible range
 			if (check_distance_bl(src, bl, AREA_SIZE))
 				clif_skill_nodamage(bl, bl, skill_id, skill_lv, 1);

+			if( skill_id == SOA_SOUL_OF_HEAVEN_AND_EARTH ){


Suggested change

if( skill_id == SOA_SOUL_OF_HEAVEN_AND_EARTH ){

if( skill_id == SOA_SOUL_OF_HEAVEN_AND_EARTH ){

status_percent_heal( bl, 0, 100 );

Missing SP healing of target.

Lemongrass3110 · 2024-03-11T23:21:06Z

src/map/skill.cpp

 		if (sd == NULL || sd->status.party_id == 0 || (flag & 1)) {

 			// Animations don't play when outside visible range
 			if (check_distance_bl(src, bl, AREA_SIZE))
 				clif_skill_nodamage(bl, bl, skill_id, skill_lv, 1);

+			if( skill_id == SOA_SOUL_OF_HEAVEN_AND_EARTH ){
+				if( src != bl && tsc && tsc->getSCE(SC_TOTEM_OF_TUTELARY) ){


Suggested change

if( src != bl && tsc && tsc->getSCE(SC_TOTEM_OF_TUTELARY) ){

if( src != bl && sc && sc->getSCE(SC_TOTEM_OF_TUTELARY) ){

Using this skill while the USER is under the effect...

Atemo · 2024-03-11T02:23:20Z

src/map/skill.cpp

@@ -7129,6 +7147,21 @@ int skill_castend_damage_id (struct block_list* src, struct block_list *bl, uint
 			status_change_end(src, SC_INTENSIVE_AIM_COUNT);
 		break;

+	case SOA_TALISMAN_OF_BLUE_DRAGON:
+		clif_skill_nodamage(src,bl,skill_id,skill_lv,1);
+		skill_attack(BF_MAGIC,src,src,bl,skill_id,skill_lv,tick,flag);


Suggested change

skill_attack(BF_MAGIC,src,src,bl,skill_id,skill_lv,tick,flag);

skill_attack(skill_get_type(skill_id), src, src, bl, skill_id, skill_lv, tick, flag);

Atemo · 2024-03-12T19:49:54Z

db/re/skill_db.yml

+        Time: 270000
+      - Level: 5
+        Time: 300000
+    Duration2:


When is Duration2 used?

Atemo · 2024-03-12T20:07:25Z

db/re/status.yml

+    DurationLookup: SOA_TALISMAN_OF_PROTECTION
+    Flags:
+      BlEffect: true
+      DisplayPc: true


The SC already has these conditions

// If the caster is offline, dead, on another map or // if the target is not a player or is in another party // End the status change

but maybe we could also add the flag

RemoveOnMapWarp - Removed when warping to another map.

?

I guess the NoSave flag should also be defined, but that's just a guess

NoSave - Won't be saved when player logs out.

Atemo · 2024-03-12T20:11:35Z

db/re/status.yml

+      Patk: true
+    Flags:
+      BlEffect: true
+      DisplayPc: true


https://www.divine-pride.net/database/skill/5419

Can only be used on target who equips weapon.

How about using the flags

RemoveOnUnequipWeapon - Removed when unequipping a weapon. RequireWeapon - Status requires a weapon to be equipped.

Atemo · 2024-03-12T20:13:17Z

db/re/status.yml

+      Smatk: true
+    Flags:
+      BlEffect: true
+      DisplayPc: true


https://www.divine-pride.net/database/skill/5420

Can only be used on target who equips weapon.

How about using the flags

RemoveOnUnequipWeapon - Removed when unequipping a weapon. RequireWeapon - Status requires a weapon to be equipped.

Atemo · 2024-03-12T20:14:05Z

db/re/status.yml

+      All: true
+    Flags:
+      BlEffect: true
+      DisplayPc: true


https://www.divine-pride.net/database/skill/5423

Can only be used on target who equips weapon.

How about using the flags

RemoveOnUnequipWeapon - Removed when unequipping a weapon. RequireWeapon - Status requires a weapon to be equipped.

Atemo · 2024-03-12T20:21:11Z

src/map/status.cpp

+			int bonus = sc->getSCE(SC_TALISMAN_OF_FIVE_ELEMENTS)->val2;
+
+			for( const auto &element : elements ){
+				sd->indexed_bonus.magic_atk_ele[element] += bonus;


https://www.divine-pride.net/database/skill/5423

Attaches the talisman of five elements to self or 1 party member to increases physical and magical damage against water, wind, earth, fire and neutral property enemies.

For me it should be

Suggested change

sd->indexed_bonus.magic_atk_ele[element] += bonus;

sd->indexed_bonus.magic_addele[element] += bonus;

magic_addele: +x% magical damage against unit element e
while magic_atk_ele increases damage of e element magic of caster by x%

aleos89 · 2024-04-27T12:49:57Z

src/map/skill.cpp

+	case SOA_TALISMAN_OF_SOUL_STEALING:
+		skill_attack(skill_get_type(skill_id), src, src, bl, skill_id, skill_lv, tick, flag);
+		if( bl->type != BL_SKILL ){
+			int64 sp = 100 * skill_lv + status_get_lv(src);


Would it be easier to just calculate this in the function call below?

aleos89 · 2024-04-27T12:57:28Z

src/map/skill.cpp

-		name.resize(SKILL_DESC_LENGTH);
-		memcpy(skill->desc, name.c_str(), sizeof(skill->desc));
+		desc.resize(SKILL_DESC_LENGTH);
+		memcpy(skill->desc, desc.c_str(), sizeof(skill->desc));


Should we change the storage for skill names and descriptions to string to get rid of scary mem* manipulations?

Atemo · 2024-05-31T15:04:40Z

src/map/status.cpp

+
+			int hp = skill_calc_heal( &ssd->bl, bl, SOA_TALISMAN_OF_PROTECTION, sce->val1, true );
+
+			status_heal( bl, hp, 0, 0, 2 );


Heal effect (not 0 HP) is also displayed on official server when player hp is hpmax.

datawulf and others added 17 commits March 11, 2024 02:02

Implemented Soul Ascetic

ddf9270

Taken from #7024

Added checks for the length of a skill's name and description

ccb646d

Moved cases to the bottom of the switch in battle_calc_magic_attack

2b6de01

Cleaned up battle.cpp

e0ffe51

Cleaned up SOA_CIRCLE_OF_DIRECTIONS_AND_ELEMENTALS

25c7fee

Cleaned up SOA_TALISMAN_OF_BLUE_DRAGON

17c5618

Cleaned up SOA_TALISMAN_OF_SOUL_STEALING

9ab262a

Cleaned up SOA_EXORCISM_OF_MALICIOUS_SOUL

d05e09a

Cleaned up SOA_TALISMAN_OF_WHITE_TIGER

bc393a0

Cleaned up SOA_SOUL_OF_HEAVEN_AND_EARTH

235e424

Cleaned up SOA_SOUL_GATHERING

f71dba4

Cleaned up SOA_TALISMAN_OF_PROTECTION

528022a

Cleaned up SOA_TALISMAN_OF_WARRIOR & MAGICIAN & FIVE_ELEMENTS

15fbec4

Moved cases to the bottom of the switch in skill_castend_nodamage_id

94e425d

Moved cases to the bottom of the switch in skill_castend_pos2

a008a7f

Cleaned up UNT_TOTEM_OF_TUTELARY

693882b

Cleaned up SC_TALISMAN_OF_PROTECTION

0e6f14f

Cleaned up script_constants.hpp

8d4abec

Lemongrass3110 commented Mar 11, 2024

View reviewed changes

Lemongrass3110 added 2 commits March 11, 2024 10:27

Apply suggestions from code review

a8ca35a

Confirmed by @datawulf

Apply suggestions from code review

8527156

Confirmed by @datawulf

Lemongrass3110 commented Mar 11, 2024

View reviewed changes

Lemongrass3110 added 3 commits March 11, 2024 21:30

Merge branch 'class/soul_ascetic' of https://github.com/rathena/rathena…

73b87c7

… into class/soul_ascetic

Merge branch 'master' into class/soul_ascetic

010e6fa

# Conflicts: # db/re/status.yml

Refactor SOA_TALISMAN_OF_PROTECTION

9afae11

Lemongrass3110 commented Mar 11, 2024

View reviewed changes

datawulf mentioned this pull request Mar 12, 2024

Updates to Initial implementation of Soul Ascetic #8169

Open

Atemo reviewed Mar 12, 2024

View reviewed changes

aleos89 reviewed Apr 27, 2024

View reviewed changes

Atemo reviewed May 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial implementation of Soul Ascetic #8162

Initial implementation of Soul Ascetic #8162

Lemongrass3110 commented Mar 11, 2024

Lemongrass3110 Mar 11, 2024

Lemongrass3110 Mar 11, 2024

Lemongrass3110 Mar 11, 2024

Lemongrass3110 Mar 11, 2024

Lemongrass3110 Mar 11, 2024

Lemongrass3110 Mar 11, 2024

Lemongrass3110 Mar 11, 2024

Lemongrass3110 Mar 11, 2024

Atemo Mar 11, 2024

Atemo Mar 12, 2024

Atemo Mar 12, 2024

Atemo Mar 12, 2024

Atemo Mar 12, 2024

Atemo Mar 12, 2024

Atemo Mar 12, 2024

aleos89 Apr 27, 2024

aleos89 Apr 27, 2024

Atemo May 31, 2024

	hp += 50 * pc_checkskill( tsd, SOA_TALISMAN_MASTERY ) * sg->skill_lv;
	hp += 50 * pc_checkskill( BL_CAST( BL_PC, ss ), SOA_TALISMAN_MASTERY ) * sg->skill_lv;

	hp += 50 * status_get_crt( ss ) * sg->skill_lv;
	hp += 5 * status_get_crt( ss );

	sp += 5 * tstatus->crt * sg->skill_lv;
	sp += 5 * tstatus->crt;

	sp *= 100 + status_get_lv( ss );
	sp /= 150;
	sp *= status_get_lv( ss ) / 100;

	if( src != bl && tsc && tsc->getSCE(SC_TOTEM_OF_TUTELARY) ){
	if( src != bl && sc && sc->getSCE(SC_TOTEM_OF_TUTELARY) ){

	skill_attack(BF_MAGIC,src,src,bl,skill_id,skill_lv,tick,flag);
	skill_attack(skill_get_type(skill_id), src, src, bl, skill_id, skill_lv, tick, flag);

	sd->indexed_bonus.magic_atk_ele[element] += bonus;
	sd->indexed_bonus.magic_addele[element] += bonus;


		int hp = skill_calc_heal( &ssd->bl, bl, SOA_TALISMAN_OF_PROTECTION, sce->val1, true );

		status_heal( bl, hp, 0, 0, 2 );

Initial implementation of Soul Ascetic #8162

Are you sure you want to change the base?

Initial implementation of Soul Ascetic #8162

Conversation

Lemongrass3110 commented Mar 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment