Skip to content

Conversation

@Emistry
Copy link
Member

@Emistry Emistry commented Oct 2, 2018

  • for stat reduction support.
  • return the amount of status point required to increase a status.

Pull Request Prelude

Changes Proposed

Affected Branches: Master

Issues addressed: #1234
References rathena/rathena@51ef911

Known Issues and TODO List

@Emistry Emistry added component:scripts Affecting the scripts and NPCs component:core:scriptengine Affecting the script engine or the script commands component:documentation Affecting the documentation in the doc/ folder labels Oct 2, 2018
@HerculesWSAPI
Copy link
Contributor

This change is Reviewable

src/map/script.c Outdated
int val = script_getnum(st, 3);;
struct map_session_data *sd = script->rid2sd(st);

if (sd != NULL)
Copy link
Contributor

Choose a reason for hiding this comment

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

will be script error if sd not attached.
probably better return some error status in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

wait, aren't the script->rid2sd(st) would throw the error status if sd == NULL?

Copy link
Contributor

Choose a reason for hiding this comment

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

i mean bit other. yes error in server console is fine
but current code also terminate npc script, because here no pushint in else branch.
if you add pushint also on errors, npc script can get result and continue running

Copy link
Contributor

Choose a reason for hiding this comment

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

ops, rid2sd terminating script already. then probably better replace rid2sd to map->id2sd(st->rid)

Copy link
Member Author

Choose a reason for hiding this comment

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

i think its better to remain script->rid2sd(st);.
the script command are designed for player to retrieve that status points.
Its fine to terminate the script if no player are attached to it, just like any other script commands like *statusup() etc

@Emistry Emistry force-pushed the scriptcommand_pcneedstatuspoint branch from eae4dbc to 5166976 Compare October 3, 2018 16:36
// - Returns status points equals to points needed to raise
// that stat to original value.
// - Doesn't work if base status <type> would become lower than 1 after reduction.
// * callfunc("F_CashReduceStat",<type>{,<val>,<itemid>});
Copy link
Contributor

Choose a reason for hiding this comment

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

please correct the style issues in this function, spaces after , and if and parenthesis for script command calls.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

- for stat reduction support.
- return the amount of status point required to increase a status.
@Emistry Emistry force-pushed the scriptcommand_pcneedstatuspoint branch from 5166976 to d7bbebd Compare October 4, 2018 17:33
@MishimaHaruna MishimaHaruna added this to the Release v2018.12.16 milestone Dec 16, 2018
@MishimaHaruna MishimaHaruna merged commit 52360f0 into HerculesWS:master Dec 16, 2018
@Emistry Emistry deleted the scriptcommand_pcneedstatuspoint branch April 14, 2019 09:20
@dastgirp dastgirp mentioned this pull request Jul 30, 2019
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:core:scriptengine Affecting the script engine or the script commands component:documentation Affecting the documentation in the doc/ folder component:scripts Affecting the scripts and NPCs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants