Job_Super_Novice_E is not supposed to be 3rd job

[AnnieRuru]

To Reproduce @job 4190 change job into Job_Super_Novice_E @bodystyle 1 instantly crash the client

Describe the bug Supposely only 3rd jobs can change body style, but currently Job_Super_Novice_E (4190) is consider a 3rd class job EAJ_SUPER_NOVICE_E: 0x4100 which break down as EAJL_THIRD | EAJL_2_1

Expected behavior Asheraf said in discord

@AnnieRuru that is more of a mistake with super novice, it should be in 2nd jobs category not 3rd (from decompiled aegis, job 23 is 1st class, job 4190 is second class)

System specs (please complete the following information):

• OS: Windows 7 Service Pack 1 Ultimate
• Hercules Version: 2019-2-11
• Packet version: 20180620
• Mode: Renewal
• Client type: 2018-06-20eRagexeRE

Additional context our eaclass() doesn't have EAJL_1 ... should we make a new one ? https://github.com/AnnieRuru/customs/blob/master/plugin/ea_super_novice.diff

[Rytech2]

I believe the Super Novice was coded in a way in eAthena to be the 2nd job of the Novice for some reasons. Basically to have the heart of a Novice and be recognized by NPC's and script code as one. I tried making SN 1st job and ESN 2nd job before and it lead to problems and have to revert back.

Also making this change will only temp fix the issue with the body style command. Possible crash issues will appear again once you add in support for the Star Emperor and Soul Reaper. Its best to have a check to see if the player's current job has a 2nd body style or not instead of checking if the player is a 3rd job.

[Asheraf]

I don't see why super novice isn't made the same way the other extended classes are, nothing magical here.. and why would we want scripts to recognize a super novice as a novice?

Novice -> Gunslinger -> Rebellion
Novice -> Ninja -> Kagerou/Oboro
Novice -> Taekwon -> Star Gladiator/Soul Linker
Novice -> Super Novice -> Extended Super Novice

As for the SE and SR they are also not a 3rd job, It's an expanded class "상위 확장직업".

[Rytech2]

Kagerou/Oboro/Rebellion are also marked as expanded classes and not as 2nd jobs, yet we had them coded as 2nd jobs for the longest time and it worked out well for everyone. Even aegis data shows they are 2nd's. So should they get treated differently too? Also where did you see in the aegis data that SN is a 1st job? Im looking at it now and ID 23 isn't assigned in the FirstJobCat which makes it default to 0 (Novice) as its first job and it returns its own ID in the SecondJobCat.

[Asheraf]

Then we should create a group for the expanded classes, using the same stuff as for the normal jobs makes it messy. for categories there is this two functions

bool IsFirstJob(const int in_Job)
{
    int pJob;

    pJob = GetPureJob(in_Job);
    return (unsigned int)pJob <= 6 || pJob >= 23 && (pJob <= 25 || pJob == 4046);
}

bool IsSecondJob(const int in_Job)
{
    int pJob;
    bool result;

    pJob = GetPureJob(in_Job);
    if ((unsigned int)(pJob - 7) <= 13 || (unsigned int)(pJob - 4047) <= 2)
        result = 1;
    else
        result = pJob == 4190;
    return result;
}

[Rytech2]

Could use something like this....(codebox isnt working for some reason)

define JOBL_EXPANDED 0x8000

// JOBL_EXPANDED in place of JOBL_2_1 but might break item job equip mask. // Just make these jobs use the same job equip mask as their 1st job parts. MAPID_REBELLION = JOBL_EXPANDED | MAPID_GUNSLINGER, MAPID_KAGEROUOBORO = JOBL_EXPANDED | MAPID_NINJA,

MAPID_SUPER_NOVICE_E = JOBL_EXPANDED | JOBL_2_1 | MAPID_NOVICE, MAPID_STAR_EMPEROR = JOBL_EXPANDED | JOBL_2_1 | MAPID_TAEKWON, MAPID_SOUL_REAPER = JOBL_EXPANDED | JOBL_2_1 | MAPID_TAEKWON,

[AnnieRuru]

@Asheraf rather than creating a function, we actually use bits https://github.com/HerculesWS/Hercules/blob/stable/src/map/map.h#L243

and why nobody think adding a 1st job bits ? JOBL_1 0x0400

@Rytech2 your solution doesn't fix the super novice being read as 2nd job though super novice should be 1st job ... right ? <-- correct me if I'm wrong

[hemagx]

@AnnieRuru what @Asheraf posted is actual decompiled functions from Aegis and not suggesting to make such functions. In other words, It's confirmed that even Aegis server side treat it as 2nd job. The PureJob function would return 23 for job 4045 (Super Novice), and 4190 for (Extended Super novice), and as Asheraf posted the first job function would indeed be true for Super Novice and second job function would be indeed true for Extended Super Novice. So the problem here is we marking it as 3rd job, as simple as that. Also, @Rytech2 it has nothing to do with how the client is displaying it or the item code groups, item code groups in Aegis is treating all expanded jobs as individual groups that has nothing to do with 1st/2nd/3rd jobs. and last, for the "Long function" you mentioned, you're just confusing the function that returns the base job for each job, with actual category functions.

[Rytech2]

Im pretty sure I know what im talking about but ok. ill just step out of this convo and let you guys be.

Off Topic: The hell is up with Github? Edit isnt working but clicking delete straight up deletes a post without confirm??? Ugh. Sorry I didn't mean to delete my previous post.

[AnnieRuru]

ok so if just only want to fix the bodystyle issue, we can just do like ...

 src/map/pc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/map/pc.c b/src/map/pc.c
index ea18715bb..9c67ddb08 100644
--- a/src/map/pc.c
+++ b/src/map/pc.c
@@ -12335,7 +12335,7 @@ static bool pc_has_second_costume(struct map_session_data *sd)
 {
    nullpo_retr(false, sd);

-   if ((sd->job & JOBL_THIRD) != 0)
+   if ((sd->job & JOBL_THIRD) != 0 && (sd->job & MAPID_BASEMASK) != MAPID_NOVICE)
        return true;
    return false;
 }

... right ? testing right now ... ok

[Asheraf]

@AnnieRuru you can create a pr with that temporary fix, but we're keeping this open since the source problem is not fixed.