Skip to content
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

Display Bedrock players #54

Merged
merged 4 commits into from
Aug 29, 2023
Merged

Display Bedrock players #54

merged 4 commits into from
Aug 29, 2023

Conversation

Dantevg
Copy link
Owner

@Dantevg Dantevg commented Dec 4, 2022

Closes #53

@Dantevg Dantevg linked an issue Dec 4, 2022 that may be closed by this pull request
@Olen
Copy link
Contributor

Olen commented Aug 21, 2023

Any chance of getting this in a release?

I made a minor fix to WebStats-dist.js just to get it to actually displaying some stats at all:

    static isPlayerOrServer = t => "#server" == t || t.match(/^\w{3,16}$/) && !t.match(/^\d*$/)||this.isBedrockPlayer(t);
    static isBedrockPlayer=t=>t.startsWith('.');

@Dantevg
Copy link
Owner Author

Dantevg commented Aug 21, 2023

This PR is pretty old and based on older code so it will need a bit of work to get it in a working (and mergeable) state again.

I'm happy that there is some enthusiasm for getting bedrock to work, though! The primary reason that this wasn't merged is that I cannot test whether it works or not (I do not have a bedrock server). If you would be willing to test, I can probably get a version working somewhere this week.

@Olen
Copy link
Contributor

Olen commented Aug 21, 2023

I am more than happy to test it.
And the code I pasted above was only the bare minimum from the original PR that made it work (just allowing a '.' in front ot the player name).

@Dantevg Dantevg merged commit e2919b4 into master Aug 29, 2023
@Dantevg
Copy link
Owner Author

Dantevg commented Aug 29, 2023

I didn't know it was that simple :) The original PR code also included a way to change the bedrock prefix (because apparently it is configurable), but for simplicity I decided to rip out that code. Should be working as it is basically just your fix as well!

@Dantevg Dantevg deleted the bedrock-players branch August 29, 2023 19:31
@Olen
Copy link
Contributor

Olen commented Aug 30, 2023

Do you have a test-release I can try?
Did not find an updated jar anywhere.

@Dantevg
Copy link
Owner Author

Dantevg commented Aug 30, 2023

The latest commit has a github action that automatically builds the jar and js file. You can find it on the bottom of the action summary page, under "Artifacts".

@Olen
Copy link
Contributor

Olen commented Aug 30, 2023

Thanks. Found it now, and it seems to work.
For some reason, the WebStats-dist.js included in the zip-file from the action summary, did not include the fix.
I believe it might be because the one in src/main/resources is still the old one, while the one directly in web/ seems to have the fix added.

edit
Sorry. None of them contains the BEDROCK_PREFIX But the one I got when buidling it myself using npm had it...

@Dantevg
Copy link
Owner Author

Dantevg commented Aug 30, 2023

Damn, I thought I had tested the github actions thing properly but apparently not... For what it's worth, here's the jar including the js file: WebStats-1.8.6-dev.zip

EDIT: the newest commits have fixed the github actions artifact output: https://github.com/Dantevg/WebStats/actions/runs/6023007759

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bedrock Players are not listed
2 participants