New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quest and Questitem extensions #33

Merged
merged 2 commits into from Jan 11, 2018

Conversation

Projects
None yet
3 participants
@cljnilsson
Copy link
Contributor

cljnilsson commented Jan 8, 2018

Nuff said.

@Frotty
Copy link
Member

Frotty left a comment

cool, that was quick :)
Please fix the compile error (All checks have failed -> view details) and 2 other small things, then I will merge asap.

@@ -0,0 +1,24 @@
package QuestItem

public function questitem.description(string text)

This comment has been minimized.

@Frotty

Frotty Jan 8, 2018

Member

Shouldn't this be "setDescription" like the other ones?

DestroyQuest(this)


public enum QuestState

This comment has been minimized.

@Frotty

Frotty Jan 8, 2018

Member

"public enum QuestState" should not be indented, this is causing the travis build to fail

public function quest.isCompleted() returns boolean
return IsQuestCompleted(this)

public function quest.flash()

This comment has been minimized.

@Frotty

Frotty Jan 8, 2018

Member

This shouldn't be an extension function, or?

@cljnilsson

This comment has been minimized.

Copy link
Contributor Author

cljnilsson commented Jan 8, 2018

I think it should be an extension unless I am missing something entirely. Everything else is fixed.

@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Jan 8, 2018

FlashQuestDialogButton doesn't take a quest handle though. And hence it doesn't change whatever quest you call it upon. It's mostly confusing.

return qi.isCompleted()

function complete(boolean b)
qi.complete(b)

This comment has been minimized.

@Cokemonkey11

Cokemonkey11 Jan 8, 2018

Contributor

missing eof lf

@cljnilsson

This comment has been minimized.

Copy link
Contributor Author

cljnilsson commented Jan 9, 2018

Ohhh. Good point.

@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Jan 9, 2018

so will you push a fix?? 😆

@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Jan 11, 2018

I guess not. Well thanks anyway 👍

@Frotty Frotty merged commit ac5d099 into wurstscript:master Jan 11, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment