-
Notifications
You must be signed in to change notification settings - Fork 29
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
Conversation
…Team/lodestone into 145-0.4.3-player-list
… resolution of avatar
… resolution of avatar
…Team/lodestone into 145-0.4.3-player-list
✅ Deploy Preview for lodestone-dashboard ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for lodestone-storybook ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove these comments
There was a problem hiding this 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: ''} |
There was a problem hiding this comment.
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
also the icon looks a bit small... can you @Arcslogger take a look next week? |
…t neighbour on avatar, cleanup
There was a problem hiding this 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
backend will follow up once the configurable refactor is merged in |
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.
If there is no one online, the text changes to say "No players online".
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
How Has This Been Tested?
Extremely thorough testing B)