Issue information

Issue ID
#6689
Status
Fixed
Severity
None
Started
Hercules Elf Bot
Sep 13, 2012 17:40
Last Post
Ind
Jan 27, 2013 11:36
Confirmation
N/A

Hercules Elf Bot - Sep 13, 2012 17:40

Originally posted by [b]mkbu95[/b]
I was looking into @feelreset and there was no verification regarding to being Star Gladiator.
But then I thought, oh, you can use @job, and you might wanna keep the "save maps" when you were with Star Gladiator job.
And again I thought (I can do it pretty much anytime /slur ), what if you reset it when you jobchange, so you would make things a little more correct?

So, I made this diff, i'm not sure if it is a bug (if it's intended to save maps regardless your job and reset it [even if you don't have any]).

Also, I renamed the command to resetfeel to match with the official one /resetfeel /gg


Here: [attachment=3526:resetfeel.patch]

Regards,
mkbu95

Hercules Elf Bot - Sep 13, 2012 23:35

Originally posted by [b]Lighta[/b]
You may have mapcrash with npc using that command with this diff; I woul add a nullpochk on sd to prevent it at least.
Otherwise ok for enforce the check but I think they'll prefer the match with MAPID

Hercules Elf Bot - Sep 14, 2012 1:14

Originally posted by [b]mkbu95[/b]
I don't think would need that. Since it's an atcommand, the char is the one to invoke the command.
And why is good to use MAPID?
I only found that [code]case JOB_STAR_GLADIATOR: return MAPID_STAR_GLADIATOR;[/code] and vice-versa.
Is there any perfomance effect between [code]job == JOB_STAR_GLADIATOR[/code] and [code]job&MAPIP_STAR_GLADIATOR[/code] ?

Hercules Elf Bot - Sep 16, 2012 16:24

Originally posted by [b]Lighta[/b]
Yes performance wise the 2nd is faster I think, since the 1st one depend on mmo_status witch is fetched from char...
The 1st one is the classid for the client mostly.
An atcommand can be called without a player attached :
[code]
OnClock0010: //call me without a player attached
atcommand "@feelreset"; //mapcrash du to segfault
[/code]

Hercules Elf Bot - Sep 17, 2012 3:03

Originally posted by [b]mkbu95[/b]
Hmm, forgot about that XD
There are other commands without nullpo_retr :o

Hercules Elf Bot - Oct 9, 2012 4:06

Originally posted by [b]Vianna[/b]
Atcommands don't need null pointer checks, because they'll only be invoked by players. When they're invoked by scripts, a temporary player is created to run the command.

Regarding the Job Change issue, I think Star Gladiator maps/targets should be reset when they change their jobs to anything, so there aren't (more) pointless player variables in the database.

Checking an integer for equality to JOB_STAR_GLADIATOR and for & MAPID_STAR_GLADIATOR takes almost the same time, but converting from JOB to MAPID before actually comparing would take a little longer. This comparison is done only a single time when this specific command is typed, so micro-optimizations shouldn't matter. But checking for MAPID is really better, because if Gravity adds a Star Gladiator 3rd job or baby, the condition wouldn't have to be changed.

Ind - Jan 27, 2013 11:36

Fixed in [url="https://github.com/HerculesWS/Hercules/commit/cf690d4206ea3c0e99d7f7a31951c5fce623a43f"]https://github.com/HerculesWS/Hercules/commit/cf690d4206ea3c0e99d7f7a31951c5fce623a43f[/url]