A new executor, and a renewed executor #195

rkdrns4747 · 2019-08-26T12:18:44Z

1. New executor, #KICK

This #KICK Executor is simply used to kick player.

1) kick sender - Not giving any argument

#KICK

2) kick sender with custom kick message - giving only one argument

#KICK "You are kicked"

3) kick specific player - giving only one argument

#KICK player("Pro_Snape")

4) kick specific player with custom kick message - giving two arguments

#KICK player("Pro_Snape") "You are kicked lmao"

2. Made #CLEARCHAT to accept Player instance as arg.

This makes #CLEARCHAT able to accept Player instance.

1) Clear specific player's chat

#CLEARCHAT player("Pro_Snape")

This #KICK Executor simply use to kick player. 1) kick sender - Not giving any argument #KICK 2) kick sender with custom kick message - giving only one argument #KICK "You are kicked" 3) kick specific player - giving only one argument #KICK player("Pro_Snape") 4) kick specific player with custom kick message - giving two arguments #KICK player("Pro_Snape") "You are kicked lmao" Tested.

This commit makes #CLEARCHAT able to accept Player instance. **1)** Clear specific player's chat #CLEARCHAT player("Pro_Snape")

gerzytet · 2019-08-26T14:53:10Z

The executors are looking good!
To make these executors more consistent with the style of the other executors, I have some suggestions:

Add a validation check to reject argument lists that are too long, such as more than 2 args for #KICK
When an argument has the wrong type, always print the argument itself in the error message rather than its type, so that it’s value is shown and users don’t ask us what a org.bukkit.Location is.
Avoid chat colors in errors. It’s better to let the error handling within the interpreter format it for you. If you are interested in making prettier error messages, you should change the handling itself so that all error messages can benefit.

gerzytet · 2019-08-26T14:53:31Z

Sorry about the close-reopen, that was an accident

print error if there are more than two arguments.

add validation check when args's length is more than 2.

rkdrns4747 · 2019-08-26T15:22:57Z

Sorry about the close-reopen, that was an accident

Fixed what you're recommended. Hope these Executors work perfect.

wysohn · 2019-08-26T19:14:01Z

@rkdrns4747
Thank you for the pull request!

Do you also mind to write test cases for this PR?

You can reference link

Actually, you will edit AbstractTestExecutors.java and write test codes just like the other few examples in it.

If you are able to do that, I would have no problem adding you to Collaborator.

Also, briefly learn about how 'git' works too! it will help you a lot.

same as title.

just knew eq() and anyString() don't work, also some elses.

some error fixed. Mokito -> Mockito

same as last commit, Mokito -> Mockito

wysohn · 2019-08-27T05:36:44Z

@rkdrns4747
Keep up the good work!

Please also be advised that we use 4 spaces instead of tab for indentation.

rkdrns4747 · 2019-08-27T05:56:14Z

@wysohn
kk, got it.
btw I got error while testing executor in Trivis, but I cannot see details of the error. Is there any way to see detailed information about an error occured while testing an Executor?

wysohn · 2019-08-27T06:25:22Z

@rkdrns4747
If you are on Intellij, your test folder will be in green color

right click on it then you can run the test in your local machine (the run button will say something like 'run test blah blah')

it will throw error in there

ECMAException is usually caused directly by running javascript, so make sure that your assertError() are well implemented.

rkdrns4747 · 2019-08-27T17:02:32Z

@wyshon
I ran gradle build task in IntelliJ, but It says build finished without any errors.
Is this possible thing that build result is differrent between travis CI's build and IntelliJ's build?
(Also, I found "run test ... gger reactor" menu but It says "No tasks available.")

wysohn · 2019-08-27T17:30:51Z

What version of jdk are you using?

It should be 1.8 though

rkdrns4747 · 2019-08-27T22:29:45Z

@wysohn

yea I'm using jdk1.8. and also project sdk is jdk 1.8

wysohn · 2019-08-28T00:56:26Z

maybe directly right click on the java file instead of folder

we need to know what that ECMAException is caused by

rkdrns4747 · 2019-08-28T02:02:45Z

@wysohn kk. I'll try that after my school ended.

final one

same as title.

rkdrns4747 · 2019-08-28T13:01:55Z

ahh, now I know how Mockito works. thanks for help @wysohn!

same as title

rkdrns4747 · 2019-08-28T13:55:39Z

@wysohn
Can I ask you help one more? I got BUILD COMPLETE on IntelliJ, but I got error while building on Travis, ParserExeception.

this is what I got.

https://imgur.com/a/Q7uU0gx

gerzytet · 2019-08-28T17:37:54Z

When you’re using assertError, the error should be pre-computed to minimize any suprises
For example, use:
"Invalid world: 34"
Not:
"Invalid world: " + arg0

Also, when you test for equality in your JavaScript, make sure to use === instead of == for equality testing. == on some types can have surprising results. ("1" == 1 is true, 0 == "" is true, etc) === is less error prone

rkdrns4747 · 2019-08-28T23:00:16Z

@gerzytet ohh, that's why I don't feel good when I make new Executor, PlaceHolder. Thanks for reminding :)

also, I'll fix them a few hours later.

same as title.

fix == to ===.

wysohn · 2019-08-29T04:51:46Z

There must be a way in Intellij to test it locally

At least you can build it with gradle then it will throw errors in console as messages

We can't do much but assume that ECMAExeption is indeed thrown by the KICK executor itself, but we never know the details.

At least the code seems to be working, but maybe the message part of asserError() is not exactly matching with the one in the test cases.

rkdrns4747 · 2019-08-29T05:54:38Z

@wysohn I got passed KICK.js here test on Travis too, but my the actual problem is testClearChat() one that I recently added.
If assertError() 's message doesn't matched to Executor message well, It must have given me error when I build on my local device.

wysohn · 2019-08-29T07:19:22Z

Apparently, that's a new error

If ParserException is thrown, I assume you have some syntax error in the CLEARCHAT executor

It's not related to the test

rkdrns4747 · 2019-08-29T07:51:46Z

@wysohn oh kk, I'll check it thx

same as title.

wysohn · 2019-08-30T06:09:50Z

Looks good to me!

Great work

gerzytet

I’ve already kinda reviewed it, but ok

There are some stray == in clearchat
you forgot to prefix some JS variables with var (recommended for scope reasons)
eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee

same as title.

wysohn · 2019-09-18T06:38:07Z

@rkdrns4747
Please also update Wiki accordingly.

rkdrns4747 · 2019-10-09T09:46:57Z

Moved done, due to being published in ver2.2.1

wysohn and others added 3 commits August 26, 2019 00:01

Fix for #TP not allowing other arguments than x,y,z coordinates

494d9e0

Made #CLEARCHAT to accept Player instance as arg.

1bcaea7

This commit makes #CLEARCHAT able to accept Player instance. **1)** Clear specific player's chat #CLEARCHAT player("Pro_Snape")

gerzytet closed this Aug 26, 2019

gerzytet reopened this Aug 26, 2019

rkdrns4747 added 2 commits August 27, 2019 00:10

error fixes

cfa7b27

print error if there are more than two arguments.

error fixes

8252ded

add validation check when args's length is more than 2.

wysohn added the domain:suggestion A suggestion, i.e: new ideas, optimization, etc. label Aug 26, 2019

wysohn added this to the 3.0.0 milestone Aug 26, 2019

wysohn added this to In progress in 3.0.x via automation Aug 26, 2019

wysohn changed the base branch from development to 3.0.0 August 26, 2019 19:07

rkdrns4747 added 5 commits August 27, 2019 13:29

Add some test of KICK Executor

dc10970

same as title.

error fixes

2f4aae5

same as title.

error fixes

ab9f7a3

just knew eq() and anyString() don't work, also some elses.

some errors keep fixed

75953c9

some error fixed. Mokito -> Mockito

one more errror.

7ac58f4

same as last commit, Mokito -> Mockito

rkdrns4747 added 2 commits August 28, 2019 21:50

Final KICK.js error fix

6914277

final one

final KICK.js test script fix

f8738a1

same as title.

rkdrns4747 added 2 commits August 28, 2019 22:36

final CLEARCHAT.js test script

2a3d8c5

same as title

times() is static one, fixed error

5e9616b

same as title

rkdrns4747 added 2 commits August 29, 2019 10:05

add some more test, also fixed what gerzytet said.

dcde93c

same as title.

fix what gerzytet said.

e8bb2ce

fix == to ===.

rkdrns4747 added 3 commits August 29, 2019 17:13

Error fixes

d5d5ac5

Added one more test for ClearChat

e44ec6f

same as title.

Actually OfflinePlayer does not needed to be imported.

568e538

wysohn requested a review from gerzytet August 30, 2019 06:09

wysohn moved this from In progress to Testing in 3.0.x Aug 30, 2019

gerzytet reviewed Aug 30, 2019

View reviewed changes

rkdrns4747 added 2 commits August 31, 2019 01:21

add some var prefix, and fixed stray == to ===

7fbde80

same as title.

added var prefix

404de6f

same as title.

wysohn merged commit 0ae0cac into TriggerReactor:3.0.0 Sep 18, 2019

3.0.x automation moved this from Testing to Done Sep 18, 2019

wysohn moved this from Done to Testing in 3.0.x Sep 18, 2019

wysohn assigned rkdrns4747 Sep 18, 2019

wysohn moved this from Testing to Beta in 3.0.x Sep 29, 2019

rkdrns4747 moved this from Beta to Done in 3.0.x Oct 9, 2019

rkdrns4747 deleted the patch-4 branch October 9, 2019 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A new executor, and a renewed executor #195

A new executor, and a renewed executor #195

rkdrns4747 commented Aug 26, 2019

gerzytet commented Aug 26, 2019

gerzytet commented Aug 26, 2019

rkdrns4747 commented Aug 26, 2019

wysohn commented Aug 26, 2019

wysohn commented Aug 27, 2019

rkdrns4747 commented Aug 27, 2019 •

edited

wysohn commented Aug 27, 2019 •

edited

rkdrns4747 commented Aug 27, 2019 •

edited

wysohn commented Aug 27, 2019

rkdrns4747 commented Aug 27, 2019

wysohn commented Aug 28, 2019

rkdrns4747 commented Aug 28, 2019

rkdrns4747 commented Aug 28, 2019 •

edited

rkdrns4747 commented Aug 28, 2019 •

edited

gerzytet commented Aug 28, 2019 •

edited

rkdrns4747 commented Aug 28, 2019

wysohn commented Aug 29, 2019

rkdrns4747 commented Aug 29, 2019 •

edited

wysohn commented Aug 29, 2019

rkdrns4747 commented Aug 29, 2019

wysohn commented Aug 30, 2019

gerzytet left a comment •

edited

wysohn commented Sep 18, 2019

rkdrns4747 commented Oct 9, 2019

A new executor, and a renewed executor #195

A new executor, and a renewed executor #195

Conversation

rkdrns4747 commented Aug 26, 2019

1. New executor, #KICK

This #KICK Executor is simply used to kick player.

2. Made #CLEARCHAT to accept Player instance as arg.

This makes #CLEARCHAT able to accept Player instance.

gerzytet commented Aug 26, 2019

gerzytet commented Aug 26, 2019

rkdrns4747 commented Aug 26, 2019

wysohn commented Aug 26, 2019

wysohn commented Aug 27, 2019

rkdrns4747 commented Aug 27, 2019 • edited

wysohn commented Aug 27, 2019 • edited

rkdrns4747 commented Aug 27, 2019 • edited

wysohn commented Aug 27, 2019

rkdrns4747 commented Aug 27, 2019

wysohn commented Aug 28, 2019

rkdrns4747 commented Aug 28, 2019

rkdrns4747 commented Aug 28, 2019 • edited

rkdrns4747 commented Aug 28, 2019 • edited

gerzytet commented Aug 28, 2019 • edited

rkdrns4747 commented Aug 28, 2019

wysohn commented Aug 29, 2019

rkdrns4747 commented Aug 29, 2019 • edited

wysohn commented Aug 29, 2019

rkdrns4747 commented Aug 29, 2019

wysohn commented Aug 30, 2019

gerzytet left a comment • edited

Choose a reason for hiding this comment

wysohn commented Sep 18, 2019

rkdrns4747 commented Oct 9, 2019

rkdrns4747 commented Aug 27, 2019 •

edited

wysohn commented Aug 27, 2019 •

edited

rkdrns4747 commented Aug 27, 2019 •

edited

rkdrns4747 commented Aug 28, 2019 •

edited

rkdrns4747 commented Aug 28, 2019 •

edited

gerzytet commented Aug 28, 2019 •

edited

rkdrns4747 commented Aug 29, 2019 •

edited

gerzytet left a comment •

edited