Skip to content

Conversation

@chriser-
Copy link

@chriser- chriser- commented May 13, 2017

Pull Request Prelude

Changes Proposed

Check for unit data (nd->ud != NULL) before adjust the NPC speed.
Throw warnings if nd->ud == NULL.

Issues addressed:
Prevent map-server from crashing when adjust the NPC speed.

Sample NPC Script:

-	script	dummynpc	-1,{
	OnInit:
		debugmes "set speed to 100";
		npcspeed 100;
		debugmes "set speed done";
		end;
}

Result:
sample

After fixed: throw warnings but didnt crash map-server
sample2

@HerculesWSAPI
Copy link
Contributor

This change is Reviewable

@MishimaHaruna MishimaHaruna added the status:code-review Awaiting code review label May 13, 2017
src/map/script.c Outdated
unit->bl2ud2(&nd->bl); // ensure nd->ud is safe to edit
nd->speed = speed;
nd->ud->state.speed_changed = 1;
if (nd->ud != NULL) { // nd->ud is NULL when called on an NPC which is not on a map
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably better show error message and not silently ignore it?

Copy link
Member

Choose a reason for hiding this comment

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

resolved.

@Emistry Emistry added the component:core:scriptengine Affecting the script engine or the script commands label Apr 9, 2019
@Emistry Emistry requested a review from 4144 April 9, 2019 13:38
@Emistry
Copy link
Member

Emistry commented Apr 9, 2019

btw, how do I add the co-author back to the commit message? cant seem to find the author email.

Co-authored-by: name <[email protected]>

@chriser- is this right ?

Details EDIT -- after i update the fork it became 2 commits, should I squash it together? does anything in the merge commit would break this PR?

EDIT2--
this seem fixed messed up commits for me.

git reset <commit-hash>
git reset --hard
git push --force

src/map/script.c Outdated
struct npc_data* nd;
static BUILDIN(npcspeed)
{
struct npc_data *nd;
Copy link
Contributor

Choose a reason for hiding this comment

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

nd and speed vars better use in same line with defining.
For example for nd:
struct npc_data *nd = map->id2nd(st->oid);

@4144
Copy link
Contributor

4144 commented Apr 9, 2019

also need rebase

@4144
Copy link
Contributor

4144 commented Apr 9, 2019

about coauthor, you can add anything into commit messages
if you have one commit only, can use git commit --amend
or use interactive rebase and change line for commit from pick to reword

- fix map-server crash issue.
@chriser-
Copy link
Author

chriser- commented Apr 9, 2019

If it is easier, you can just re-do the fix in master and commit it under your own name.
As long as this contribution helps, I don't care if it is merged using my commit or your own.

@Emistry
Copy link
Member

Emistry commented Apr 10, 2019

closing this due to the commits are actually done in master branch which later become troublesome to fix/solve the conflicts. Reopen a new PR here.

@Emistry Emistry closed this Apr 10, 2019
@MrKeiKun
Copy link
Contributor

MrKeiKun commented May 5, 2022

anyone wants to remove this from Stalled / Forgotten in Project Board?

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 status:code-review Awaiting code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants