Skip to content

Conversation

@iRagno
Copy link
Contributor

@iRagno iRagno commented Oct 22, 2017

Pull Request Prelude

Changes Proposed

Applied a few changes to fit script with official one and applied standardization to entire script.

Affected Branches: Master

Issues addressed: None

Known Issues and TODO List

@ghost ghost added the status:code-review Awaiting code review label Oct 22, 2017
@HerculesWSAPI
Copy link
Contributor

This change is Reviewable

next;
donpcevent(instance_npcname("Doyen Irene#sarains")+"::OnDisable");
donpcevent(instance_npcname("A girl#sarains")+"::OnDisable");
dispbottom(_("As they wander off you hear strange voices from around the corner...")), "";
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems wrong :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thank you Asheraf.

@Asheraf Asheraf added component:scripts Affecting the scripts and NPCs mode:renewal For strictly Renewal issues labels Oct 22, 2017
dali,130,107,5 script Leon the Adventurer#Sara 4_M_DST_GRAND,{ // instance CD check only at the Dimensional Device
mes("[Leon the Adventurer]");
if (BaseLevel < 99) {
mes("You know... this place doesn't seem to be safe for you. Please returnto me once you have achieved LV. 99");
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 this one return to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Just for the record, this was only focused on standardization, but I guess can also add typo fixes.

@Asheraf
Copy link
Contributor

Asheraf commented Oct 26, 2017

As for the changes in PR it looks good to me, but we should take into consideration merging the npc's with same functionality into one and using the duplicate function

@Asheraf Asheraf changed the base branch from stable to master October 26, 2017 12:32
…Standards.

- Added mesf and sprintf commands where needed.
- Added _$() and _() macros where needed.
- Added curly brackets, paragraph breaks, parentheses and spaces to fit Hercules' Standards.
- Changed numeric arguments for constants.

Signed-off-by: Ragno <[email protected]>
@AtlantisRO AtlantisRO force-pushed the Standardization-saras_memory.txt branch from 62fec82 to 683c604 Compare October 27, 2017 04:42
@iRagno
Copy link
Contributor Author

iRagno commented Oct 27, 2017

Applied requested changes and squashed commits.

I saw the script can be improved in several parts, like duplicating npcs that are identical.. But, this was only fixing standardization to the script, and leaved any structural change for the future.

@Asheraf Asheraf added this to the Release v2017.11.19 milestone Oct 27, 2017
@Asheraf Asheraf merged commit 7f74da4 into HerculesWS:master Oct 29, 2017
@ghost ghost removed the status:code-review Awaiting code review label Oct 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:scripts Affecting the scripts and NPCs mode:renewal For strictly Renewal issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants