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

#145 Implement Minecraft player list component #176

Merged
merged 22 commits into from
Mar 1, 2023

Conversation

kevinzhang03
Copy link
Member

@kevinzhang03 kevinzhang03 commented Feb 25, 2023

Description

#145

Implements a Minecraft player list component that displays the player's username and avatar picture, and updates in real time as players join or leave the server. The profile pictures are retrieved from https://mc-heads.net/#basic using the player's UUID. The NAME button above the list allows for sorting usernames in alphabetical or reverse alphabetical order.

image

If there is no one online, the text changes to say "No players online".

image

The main files that contribute to this PR are are MinecraftPlayerList.tsx and PlayerListCard.tsx. MinecraftPlayerList.tsx mainly handles player data like setting the avatar and tracking online players, while PlayerListCard.tsx contains the UI.

InstanceInfo.ts and EventStream.ts also had to be modified in some way to support Player objects and updating the list of them to reflect in the UI in real time.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Extremely thorough testing B)

minecraft-player-list-test

@netlify
Copy link

netlify bot commented Feb 25, 2023

Deploy Preview for lodestone-dashboard ready!

Name Link
🔨 Latest commit 1b18ace
🔍 Latest deploy log https://app.netlify.com/sites/lodestone-dashboard/deploys/63fef1489d084a0007a7ec56
😎 Deploy Preview https://deploy-preview-176--lodestone-dashboard.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Feb 25, 2023

Deploy Preview for lodestone-storybook ready!

Name Link
🔨 Latest commit 1b18ace
🔍 Latest deploy log https://app.netlify.com/sites/lodestone-storybook/deploys/63fef1483e7dc400086c03d8
😎 Deploy Preview https://deploy-preview-176--lodestone-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@joeywangzr joeywangzr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise lgtm

const { selectedInstance: instance } = useContext(InstanceContext);
const uuid = instance?.uuid;

// JUST FOR TESTING
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove these comments

Copy link
Member

@Ynng Ynng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall really nice, just a few small things
mostly regarding the player list use state


// Default value of playerList will be an empty array since we cannot have useState after a conditional block
const [playerList, setPlayerList] = useState([
{ type: 'MinecraftPlayer', uuid: '', name: ''}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be an empty array

src/components/Minecraft/MinecraftPlayerList.tsx Outdated Show resolved Hide resolved
src/components/Minecraft/MinecraftPlayerList.tsx Outdated Show resolved Hide resolved
@Ynng
Copy link
Member

Ynng commented Feb 25, 2023

also the icon looks a bit small... can you @Arcslogger take a look next week?

@kevinzhang03 kevinzhang03 requested a review from Ynng March 1, 2023 06:00
Copy link
Member

@Ynng Ynng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good! let's merge this pr.

@CheatCod when can we get the player_list api changed merged to the backend?
https://github.com/Lodestone-Team/lodestone_core/tree/player_list_in_instance_info

@kevinzhang03 kevinzhang03 merged commit dca1fa3 into 0.4.3 Mar 1, 2023
@kevinzhang03 kevinzhang03 deleted the 145-0.4.3-player-list branch March 1, 2023 06:43
@CheatCod
Copy link
Member

CheatCod commented Mar 2, 2023

backend will follow up once the configurable refactor is merged in

@Arcslogger Arcslogger added this to the 0.4.3 milestone Mar 4, 2023
@Arcslogger Arcslogger linked an issue Mar 4, 2023 that may be closed by this pull request
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.

Make the player list component for minecraft
5 participants