Fix issue with only lowercase player name working with PvP combat (and a BountyGoal.js) #306
Conversation
|
I'm sorry, I did not mean to include the |
bostrt
changed the title
|
Nah, it's fine. Are you still working on the bounty goal or is it done? If you're still working on it I'll wait to merge |
| const QuestGoal = require('../../../src/QuestGoal'); | ||
|
|
||
| /** | ||
| * A quest goal requiring the player to locate and/or return an NPC. |
There was a problem hiding this comment.
This is super cool even if it doesn't address the original issue ;) I think it could be a worthy contribution to the core library.
| } else { | ||
| let located = false; | ||
| const goalnpcid = this.config.npc; | ||
| if (goalnpcid != null) { |
There was a problem hiding this comment.
Should there be an error if goalNpcId is null? Also, I would suggest using goalNpcId !== null for strict equality... slightly pedantic but could prevent weird bugs that might otherwise be hard to track down.
There was a problem hiding this comment.
yeah I think what I could do is making QuestGoal have a static validate() method that is called on bootup so the server won't start if you have invalid quest configurations. Like this goal could validate that it has a goal NPC. I agree about the !== as well
There was a problem hiding this comment.
@seanohue You are correct, there's a problem if the goalnpcid is null. I'll update that with a !== null check until @shawncplus can work in a validate() method.
No description provided.