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

Introduces Summoner Class #1458

Closed
wants to merge 36 commits into from
Closed

Conversation

dastgirp
Copy link
Member

@dastgirp dastgirp commented Oct 3, 2016

  • Added Packets for Several 2015 Clients
  • Added Summoner Class
  • Implemented Almost all of the Summoner Skills
  • Updated HPMHook
  • charlog now contains additional column class
  • Added max_summoner_parameter

This change is Reviewable

… and

2015-12-16
Added New Char Creation Packet(0xa39).
Added Placeholder of JOB_SUMMONER
@dastgirp dastgirp added component:core component:databases mode:renewal labels Oct 3, 2016
@dastgirp dastgirp added this to the Match Official Content milestone Oct 3, 2016
@dastgirp dastgirp added the status:inprogress label Oct 3, 2016
@@ -1560,6 +1560,14 @@ int char_make_new_char_sql(struct char_session_data *sd, const char *name_, int
if( flag < 0 )
return flag;

switch(starting_job) {
Copy link
Contributor

@4144 4144 Oct 3, 2016

Choose a reason for hiding this comment

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

Please add space after switch

@@ -654,6 +655,9 @@ END_ZEROED_BLOCK;
/// Rune Knight Dragon
#define pc_isridingdragon(sd) ( (sd)->sc.option&OPTION_DRAGON )

/// Basic Skils Check
#define pc_basicskillcheck(sd, level) ( pc->checkskill((sd), NV_BASIC) >= (level) || pc->checkskill((sd), SU_BASIC_SKILL) )
Copy link
Contributor

@4144 4144 Oct 3, 2016

Choose a reason for hiding this comment

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

Probably better move this to function

Copy link
Member Author

@dastgirp dastgirp Oct 5, 2016

Choose a reason for hiding this comment

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

@MishimaHaruna what you think of this?
I guess macro better suited this one

@@ -9452,6 +9542,36 @@ int skill_castend_nodamage_id(struct block_list *src, struct block_list *bl, uin
clif->skill_damage(src,bl,tick, status_get_amotion(src), 0, 0, 1, skill_id, -2, BDT_SKILL);
break;

case SU_HIDE:
if (tsce) {
clif->skill_nodamage(src,bl,skill_id,skill_lv,1);
Copy link
Contributor

@4144 4144 Oct 3, 2016

Choose a reason for hiding this comment

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

please add spaces after coma

Copy link
Member Author

@dastgirp dastgirp Oct 5, 2016

Choose a reason for hiding this comment

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

Done in new commit.

return 0;
}
clif->skill_nodamage(src,bl,skill_id,skill_lv,1);
sc_start(src,bl,type,100,skill_lv,skill->get_time(skill_id,skill_lv));
Copy link
Contributor

@4144 4144 Oct 3, 2016

Choose a reason for hiding this comment

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

and here add spaces too

skill->area_temp[3] = 1;
else
skill->area_temp[3] = 0;
skill->attack(skill->get_type(sg->skill_id),ss,&src->bl,bl,sg->skill_id,sg->skill_lv,tick,0);
Copy link
Contributor

@4144 4144 Oct 3, 2016

Choose a reason for hiding this comment

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

probably add spaces here after comas too

Copy link
Member Author

@dastgirp dastgirp Oct 5, 2016

Choose a reason for hiding this comment

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

Done in new commit.

"`max_sp`, `sp`, `hair`, `hair_color`, `last_map`, `last_x`, `last_y`, `save_map`, `save_x`, `save_y`) VALUES ("
"'%d', '%d', '%s', '%d', '%d','%d', '%d', '%d', '%d', '%d', '%d', '%d', '%d','%d', '%d','%d', '%d', '%s', '%d', '%d', '%s', '%d', '%d')",
char_db, sd->account_id , slot, esc_name, start_zeny, 48, str, agi, vit, int_, dex, luk,
char_db, sd->account_id , slot, esc_name, starting_job, start_zeny, 48, str, agi, vit, int_, dex, luk,
Copy link
Member

@MishimaHaruna MishimaHaruna Oct 4, 2016

Choose a reason for hiding this comment

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

Isn't this missing a '%d' in the format string? (it adds an argument, doesn't add the matching format string placeholder)

Copy link
Member Author

@dastgirp dastgirp Oct 5, 2016

Choose a reason for hiding this comment

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

Was added in other commit.

"`max_sp`, `sp`, `hair`, `hair_color`, `last_map`, `last_x`, `last_y`, `save_map`, `save_x`, `save_y`) VALUES ("
"'%d', '%d', '%s', '%d', '%d', '%d', '%d', '%d', '%d', '%d', '%d', '%d','%d', '%d','%d', '%d', '%s', '%d', '%d', '%s', '%d', '%d')",
char_db, sd->account_id , slot, esc_name, start_zeny, str, agi, vit, int_, dex, luk,
char_db, sd->account_id , slot, esc_name, starting_job, start_zeny, str, agi, vit, int_, dex, luk,
Copy link
Member

@MishimaHaruna MishimaHaruna Oct 4, 2016

Choose a reason for hiding this comment

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

Missing the format string placeholder, as above

"VALUES (NOW(), '%s', '%d', '%d', '%d', '%s', '%d', '%d', '%d', '%d', '%d', '%d', '%d', '%d')",
charlog_db, "make new char", sd->account_id, char_id, slot, esc_name, str, agi, vit, int_, dex, luk, hair_style, hair_color))
charlog_db, "make new char", sd->account_id, char_id, slot, starting_job, esc_name, str, agi, vit, int_, dex, luk, hair_style, hair_color))
Copy link
Member

@MishimaHaruna MishimaHaruna Oct 4, 2016

Choose a reason for hiding this comment

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

Here as well, missing the format string placeholder

@@ -993,6 +993,8 @@ enum {
JOB_OBORO,
JOB_REBELLION = 4215,

JOB_SUMMONER = 4218,
Copy link
Member

@MishimaHaruna MishimaHaruna Oct 4, 2016

Choose a reason for hiding this comment

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

This is a bit dangerous as is, it needs a matching change in pc_db_checkid(), so that the values between JOB_REBELLION and JOB_SUMMONER aren't valid (preferably in this commit as not to break git bisect).

Copy link
Member Author

@dastgirp dastgirp Oct 5, 2016

Choose a reason for hiding this comment

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

Isn't pc_db_checkid already changed?

bool pc_db_checkid(unsigned int class_)
{
    return class_ < JOB_MAX_BASIC
        || (class_ >= JOB_NOVICE_HIGH    && class_ <= JOB_DARK_COLLECTOR )
        || (class_ >= JOB_RUNE_KNIGHT    && class_ <= JOB_MECHANIC_T2    )
        || (class_ >= JOB_BABY_RUNE      && class_ <= JOB_BABY_MECHANIC2 )
        || (class_ >= JOB_SUPER_NOVICE_E && class_ <= JOB_SUPER_BABY_E   )
        || (class_ >= JOB_KAGEROU        && class_ <= JOB_OBORO          )
        || (class_ == JOB_REBELLION)
        || (class_ >= JOB_SUMMONER       && class_ <  JOB_MAX            );
}

{
int area = skill->get_splash(skill_id, skill_lv);
short tmpx = 0, tmpy = 0, x1 = 0, y1 = 0;
int i;

/*
Copy link
Member

@MishimaHaruna MishimaHaruna Oct 4, 2016

Choose a reason for hiding this comment

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

Can you comment this out with #if 0 instead? (and possibly add a comment mentioning why it's disabled)

I'm asking the #if 0 because it doesn't cause the nested comment problems that /* would

@@ -9546,6 +9546,16 @@ int skill_castend_nodamage_id(struct block_list *src, struct block_list *bl, uin
clif->skill_nodamage(src,bl,skill_id,skill_lv,1);
sc_start(src,bl,type,100,skill_lv,skill->get_time(skill_id,skill_lv));
break;
case SU_TUNABELLY: {
int heal = 0;
if (dstmd != NULL || dstmd->class_ != MOBID_EMPELIUM || !mob_is_battleground(dstmd)) {
Copy link
Member

@MishimaHaruna MishimaHaruna Oct 4, 2016

Choose a reason for hiding this comment

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

Did you perhaps mean && here?

Copy link
Member Author

@dastgirp dastgirp Oct 5, 2016

Choose a reason for hiding this comment

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

idk what I meant here, reworked the TunaBelly Skill, will push it with other changes and rebase

@@ -1749,7 +1749,7 @@ int pc_calc_skilltree_normalize_job(struct map_session_data *sd)
sd->sktree.second = sd->sktree.third = 0;

// limit 1st class and above to novice job levels
if(skill_point < novice_skills) {
if(skill_point < novice_skills && (sd->class_&MAPID_BASEMASK) != MAPID_SUMMONER) {
Copy link
Member

@MishimaHaruna MishimaHaruna Oct 4, 2016

Choose a reason for hiding this comment

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

If possible, please later reorder/squash this into the commit that adds support for this job

Copy link
Member Author

@dastgirp dastgirp Oct 5, 2016

Choose a reason for hiding this comment

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

Done

@@ -1622,7 +1622,7 @@ int char_make_new_char_sql(struct char_session_data *sd, const char *name_, int
if (chr->enable_logs) {
if (SQL_ERROR == SQL->Query(inter->sql_handle,
"INSERT INTO `%s` (`time`, `char_msg`, `account_id`, `char_id`, `char_num`, `class`, `name`, `str`, `agi`, `vit`, `int`, `dex`, `luk`, `hair`, `hair_color`)"
"VALUES (NOW(), '%s', '%d', '%d', '%d', '%s', '%d', '%d', '%d', '%d', '%d', '%d', '%d', '%d')",
"VALUES (NOW(), '%s', '%d', '%d', '%d', '%d', '%s', '%d', '%d', '%d', '%d', '%d', '%d', '%d', '%d')",
Copy link
Member

@MishimaHaruna MishimaHaruna Oct 4, 2016

Choose a reason for hiding this comment

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

Okay, nevermind my previous comment on the missing '%d' (please squash it into the other commit)

Copy link
Member Author

@dastgirp dastgirp Oct 5, 2016

Choose a reason for hiding this comment

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

Done, rebased, will push after all other review work is done

@dastgirp
Copy link
Member Author

@dastgirp dastgirp commented Oct 5, 2016

@MishimaHaruna @4144 Everything done, except pc_basicskillcheck changes, which I think is better as macro instead of function, or is there any reason to have it as function rather than macro?

@dastgirp
Copy link
Member Author

@dastgirp dastgirp commented Oct 5, 2016

Done with pc_basicskillcheck macro to function too :)

Copy link
Contributor

@hemagx hemagx left a comment

Items scripts should follow codding style as well, please fix the spaces in the formulas etc.
there's many null checks that done directly as in if (sd)
please fix all of those to != NULL or == NULL depends on the check, you can change sc checks too.

HPTable:[ 40, 69, 80, 92, 105, 119, 134, 150, 167, 185, // 1 - 10
204, 224, 245, 268, 291, 315, 341, 367, 395, 423, // 11 - 20
453, 484, 515, 548, 582, 617, 653, 690, 728, 767, // 21 - 30
807, 848, 890, 934, 978, 1023, 1070, 1117, 1166, 1215, // 31 - 40
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

Spaces and format here is a bit weird, i guess some editor fail.

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Fixed, and will be pushed once other reviews are done

Script: <"
.@r = getrefine();
bonus bMaxSP,10*(.@r/3)+50;
bonus bMatk,10*(.@r/3);
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

Space eg. 10 * (.@r / 3) + 50;

Copy link
Member

@MishimaHaruna MishimaHaruna Oct 7, 2016

Choose a reason for hiding this comment

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

Side note: if storing it to .@r was done due to performance concerns, please be aware that it's not really necessary (see #1441 for reference and benchmarks). You can make multiple calls to getrefine() in the same script (up to 12ish, apparently) and have better performance

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Thanks for this, didn't knew about it...

.@r = getrefine();
bonus bMatkRate,2*(.@r/3);
bonus bMaxSPrate,.@r/3;
if(.@r>=7) {
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

space after if

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Done

if(.@r>=7) {
.@r = min(.@r,10)-7;
bonus bWeaponAtkRate,(20*.@r)+40;
bonus bWeaponMatkRate,(20*.@r)+40;
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

Space eg. 10 * (.@r / 3) + 50;

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Done

if(.@r>=7) {
.@r = min(.@r,10)-7;
bonus bWeaponAtkRate,(20*.@r)+40;
bonus bWeaponMatkRate,(20*.@r)+40;
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

Space eg. 10 * (.@r / 3) + 50;

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Done

.@r = getrefine();
bonus bDex,.@r;
bonus bInt,.@r;
if(.@r > 9) bonus bUseSPrate,-5;
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

space after if

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Done

HPTable:[ 40, 69, 80, 92, 105, 119, 134, 150, 167, 185, // 1 - 10
204, 224, 245, 268, 291, 315, 341, 367, 395, 423, // 11 - 20
453, 484, 515, 548, 582, 617, 653, 690, 728, 767, // 21 - 30
807, 848, 890, 934, 978, 1023, 1070, 1117, 1166, 1215, // 31 - 40
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

Spaces are not correct, editor fail probably.

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Indeed, editor fail. Done Changing this

} else {
damage = -sce->val2;
}
if (/*(--sce->val3) <= 0 ||*/ (sce->val2 <= 0)) {
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

/(--sce->val3) <= 0 ||/ it would be better if this comment out of the if with explain of it.

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

For Hits (We don't know if it gives hit on official), probably not

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Removed the val3 changes

@@ -5199,8 +5248,16 @@ struct Damage battle_calc_weapon_attack(struct block_list *src,struct block_list
if (hd != NULL)
ATK_ADD(hd->homunculus.spiritball * 3);
}
if ((wd.flag&(BF_LONG|BF_MAGIC)) == BF_LONG) {
if (sd && pc->checkskill(sd, SU_POWEROFLIFE) > 0) {
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

sd != NULL instead

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Done

@@ -9822,7 +9822,7 @@ void clif_parse_QuitGame(int fd, struct map_session_data *sd) __attribute__((non
void clif_parse_QuitGame(int fd, struct map_session_data *sd)
{
/* Rovert's prevent logout option fixed [Valaris] */
if( !sd->sc.data[SC_CLOAKING] && !sd->sc.data[SC_HIDING] && !sd->sc.data[SC_CHASEWALK] && !sd->sc.data[SC_CLOAKINGEXCEED] && !sd->sc.data[SC__INVISIBILITY] &&
if( !sd->sc.data[SC_CLOAKING] && !sd->sc.data[SC_HIDING] && !sd->sc.data[SC_CHASEWALK] && !sd->sc.data[SC_CLOAKINGEXCEED] && !sd->sc.data[SC__INVISIBILITY] && !sd->sc.data[SC_SUHIDE] &&
Copy link
Contributor

@hemagx hemagx Oct 7, 2016

Choose a reason for hiding this comment

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

please fix the if spaces to be if (!sd

Copy link
Member Author

@dastgirp dastgirp Oct 8, 2016

Choose a reason for hiding this comment

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

Done

4144
4144 approved these changes Oct 7, 2016
dastgirp added 14 commits Oct 8, 2016
(Only Placeholder, other things related to summoner will follow-up soon)

Added SQL-Upgrade: Added `class` column in charlog
Added Summoner SC ID's in status.h
Added Function for Basic Skills check.
Lv.1: Atk+ 200%
BaseLevel 30: Adds a Chance to Reactive the skill.
Every 30 Base Levels: Increases the chance to reactive the skill.
Transforms into Bush.
Max Level: 3
Attack Increases by 50+(50+Level)%
Base Level >= 30: Activates a Chance to cast skill again.
Every 30 Base Level: Increases the chance to cast skill again.
Reduces Incoming Damange by 90% for 6 seconds.
When Knock Back, the effect disappears.
Skill Cannot be Used on GvG/Battlegrounds.
Cast time of Emergency Call doubles.
Moves to Another Position by performing High Jump:
Lv 1: Move 6 cells
Lv 2: Move 10 cells
Lv 3: Move 14 cells
MaxHP + 1000, MaxSP + 100.
Show's Spirit of Sea, Land and Life around the sprite when skill is
learned.
Consumes Fresh Shrimp to recover HP for 2 Minutes.
Max Level 5:
Recovers Every (11-SkillLevel) Seconds.
Increases INT by 20,
If More than 20 skill points invested in plant based Skills, MATK+20%.
Silvervine Steam Spear:
10% Bleeding Chance, 700% Matk at all levels.
Lv 1: Earth Magic
Lv 2: Fire Magic
Lv 3: Water Magic
Lv 4: Wind Magic
Lv 5: Ghost Magic
Base Level 30: Chance to activate skill again.
Every 30 Base Level: Increases the chance to activate skill.
Consumes 1 Catnip Fruit.
Lv 1-2: 3x3 AoE
Lv 3-4: 5x5 AoE
Lv 5: 7x7 AoE
Lasts for (2+SkillLevel) Seconds
Increases Natural Recovery of HP/SP.
Reduces Atk and MAtk by 50%
Reduces Movement Speed.
Catnip Meteor:
Increases Matk by 200+100*SkillLevel%.
When 1 Catnip is consumed, Adds a Chance to curse target[Not Implemented].
dastgirp added 21 commits Oct 8, 2016
Increases Flee, Hit and CRI by 20.
If >= 20 Skill points invested in animal-based skills, Ranged Physical
Attack + 20%.
Duration: (5+(2*SkillLv)) seconds.
Cannot be used on Boss Monsters. Effect is cancelled when Heaven's Drive
or Trample is used.
Enemies trapped on roots receive poison property damage.
Heal,Cure,Clearance cancels the effect.
Atk + 100*SkillLv%.
Reduces Fixed Amount of MaxHP for 9 seconds.
For Every 30 Base Levels, Adds an Additional chance that skill will be
activated again.
ATK + (200+100*SkillLv)%
Every 30 Base Levels, Adds an Additional Chance to reactivate the skill.
When the Enemy has Less than 50% HP Left, Damage is doubled.
For (50+10*SkillLv) Seconds, Atk +(15+5*SkillLv). Increases Movement
Speed.
If Target is Doram Race, Increases Range Physical attack by 10%.
Atk +(200+100*SkillLv)%.
When 1 Carrot is consumed, Add's a chance to stun enemy.
Lv1-2: 3x3 AoE
Lv3-4: 5x5 AoE
Lv5: 7x7 AoE
Added Item Constants in itemdb.h
Increases Heal Effect by 10%.
if >= 20 skill points are invested in seafood-based skills, Increases Heal
Effect by 20%.
Restores 10% MaxHP.
Additionally Restores 20*(SkillLv-1)% MaxHP
Protects the target for 30 seconds.
Tuna's Defense Power: (10+(20*(SkillLv-1)))% of Caster's MaxHP
Lasts for (30+30*SkillLv) Seconds.
Consumes 1 Shrimp.
Increases ATK and MATK by 10% for limited time.
Cannot be reset by dispell.
Cannot be healed by item/NPC once in BITESCAR.
Heal Skill would end the BiteScar Effect.
Increases BaseAtk and MAtk by 10%
Implemented SC_CATNIPPOWDER:
	Increases WAtk%, MAtk% and Reduces Movement Speed.
	Increases Natural Hp/Sp Recovery
Default max parameter for summoner is 120.
hemagx
hemagx approved these changes Oct 8, 2016
@dastgirp dastgirp added codereview:accepted and removed status:inprogress labels Oct 14, 2016
@hemagx
Copy link
Contributor

@hemagx hemagx commented Oct 19, 2016

@dastgir please squash the extra fixing commits and probably rebase the branceh and make HPMHooking update is last

hemagx added a commit that referenced this issue Oct 22, 2016
@hemagx
Copy link
Contributor

@hemagx hemagx commented Oct 22, 2016

Ooops forgot to add the close to my commit.
Merged through f072f3c

@hemagx hemagx closed this Oct 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codereview:accepted component:core component:databases mode:renewal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants