search

gekware / minecraft-server-hibernation

Autostart and stop minecraft-server when players join/leave
GNU General Public License v3.0
379 stars 37 forks source link

adjust list check for essentialsX Plugin users #210

Closed A-wels closed 1 year ago

A-wels commented 1 year ago

This fixes #209 EssentialsX adds a second line of output to the list command, which prevents the current approach from working.

I added a check to the function getPlayersByListCom() which looks out for the EssentialsX plugin

gekigek99 commented 1 year ago

Hi! thanks for PR

I see 2 problems tho: 1) the recognition for the plugin must be improved to be more relieable (easy todo) 2) in the test you wrote there is a /n to represent a newline, but maybe they are 2 different logs (difficult to) -> waiting on #209 for full log containing ms logs

(for both of them I'm waiting to see the full log containing ms logs)

gekigek99 commented 1 year ago

please don't commit binaries as it will make the repository unnecessarily heavy

A-wels commented 1 year ago

I added the log in this comment Did I commit a binary? Sorry for that :( However, I can't seem to find the binary in my commit. Would you mind telling me where I added it?

  1. What do you mean exactly by making the recognition more reliable? Which cases should I add?

  2. I printed the content of the output variable, and the output was:

    » [12:16:32 INFO]: [Essentials] CONSOLE issued server command: /list 
[12:16:32 INFO]: Es sind 0 von maximal 15 Spielern online.

The two lines are separated by a \n

gekigek99 commented 1 year ago

However, I can't seem to find the binary in my commit. Would you mind telling me where I added it?

It's called msh, i removed it in the next commit of the pr. I will improve the gitignore fir that.

1) idk using [Essentials] or even : [Essentials]

2) don't be fooled by seeing 2 lines in the msh logs, the function to get players by list command considers only the last line of output. Now probably it works in the test you have written... But does it work also in production?

For now dont do anything, i'll test it myself with the plugin to see how it works.

Cause we could also write a better regex to make it fail when reading essentials stupid log and use the connection count which is as reliable.

(And it's weird that in you log the server info is not retrieved as that is the first method that should be used)

gekigek99 commented 1 year ago

immagine

apparently yeah, output variable contains 2 lines separated by \n that were not splitted by msh, so you were right

gekigek99 commented 1 year ago

with last commit i removed your trickery to find plugins as we should aim at generalizing the code before making it plugin specific.

other than that, good call the issue.

If you are ok i can merge the PR

p.s. I need to improve the test a bit but first I want to merge the PR

A-wels commented 1 year ago

Your solution is much cleaner than mine! I guess you tested it on your device? Then you can of course merge the pr.

Thanks for you work :)