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

Rework Mobile #748

Merged
merged 17 commits into from
Dec 29, 2024
Merged

Rework Mobile #748

merged 17 commits into from
Dec 29, 2024

Conversation

Jack-Papel
Copy link
Collaborator

No description provided.

@Jack-Papel Jack-Papel marked this pull request as ready for review December 28, 2024 09:50
Copy link
Collaborator

@ItsSammyM ItsSammyM left a comment

Choose a reason for hiding this comment

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

image
I tracked it down to about line 560 in game manager on the client
I think it never makes it to the return statement
I have no idea why
I have no idea why it doesnt work on my machine
I tried replacing the Server.ws.onerror with Server.ws.addEventListener and it didnt seem to work

@@ -199,9 +199,9 @@ export function OutlineListSelector(props: {

return <section className="graveyard-menu-colors selector-section">
<h2>{translate("menu.lobby.roleList")}: {roleList.length}</h2>
<button disabled={props.disabled} onClick={simplify}>
{!props.disabled && <button onClick={simplify}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

i would always slap a === true or === false on these and also capital b Button not button

Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick yknow

bothAreObjects &&
Object.keys(a).length === Object.keys(b).length &&
Object.entries(a).every(([k, v]) => deepEqual(v, b[k as keyof T]))
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont know why you need Boolean here and i dont know what it does

@@ -39,7 +55,7 @@ export function useGameState<T>(
usePacketListener((type?: StateEventType) => {
if (GAME_MANAGER.state.stateType === "game" && (events ?? []).includes(type as StateEventType)) {
const value = getValue(GAME_MANAGER.state);
if (value !== state) {
if (!deepEqual(value, state)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

based and red pilled

window.addEventListener("beforeunload", onBeforeUnload);
return () => window.removeEventListener("beforeunload", onBeforeUnload);
}, [])

Copy link
Collaborator

Choose a reason for hiding this comment

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

i dont know why this is here or what it does

Copy link
Collaborator

Choose a reason for hiding this comment

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

i guess i should look

border: .13rem solid var(--primary-border-color);
border-bottom-color: var(--primary-border-shadow-color);
border-right-color: var(--primary-border-shadow-color);
border-radius: 1rem;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

image
settings menu is too small on my screen

@@ -38,6 +41,7 @@ export default function SpectatorGameScreen(): ReactElement {
<HeaderMenu chatMenuNotification={false}/>
</div>
<GameScreenMenus />
{mobile && <MenuButtons chatMenuNotification={false}/>}
Copy link
Collaborator

Choose a reason for hiding this comment

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

mobile === true if your really based

sammy when he doesnt know what comments to leave

@@ -24,6 +24,7 @@ pub struct LobbyClient{
}

#[derive(Clone, Debug, Serialize, PartialEq, Eq)]
#[serde(rename_all = "camelCase")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

i am so confused why this wasnt causing problems before

@@ -68,7 +68,7 @@ impl Lobby {
return
};

let text = text.trim_newline().trim_whitespace().truncate(100).truncate_lines(1);
let text = text.trim_newline().trim_whitespace().truncate(100);
Copy link
Collaborator

Choose a reason for hiding this comment

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

i like this change

@@ -164,6 +194,15 @@ export default function GameScreen(): ReactElement {
["addChatMessages"]
)!;

useEffect(() => {
const onBeforeUnload = (e: BeforeUnloadEvent) => {
if (!DEV_ENV) e.preventDefault()
Copy link
Collaborator

Choose a reason for hiding this comment

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

ok i see
I looked this up and couldnt test it because i couldnt even get in a lobby
I see the idea
Its weird that this is how you call that, can we put our own custom text?
I would like it so that if you confirm that you want to leave it sends a message to the server saying that you left
This way it kills your player in game

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so, and I don't think there's a way to make code run when they confirm.
Maybe if you do a window.confirm? But we can do that later

Copy link
Collaborator

@ItsSammyM ItsSammyM left a comment

Choose a reason for hiding this comment

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

Why is encyclopedia gone when i am on mobile? I disagree with this.
Lobby menu is too small horizontally on desktop.
Settings menu is too small horizontally on desktop.
Encyclopedia menu is too small horizontally on desktop, talking about the cover card version.
I'm assuming you set up the max menu default count on mobile to be 2, but I didn't test that

@Jack-Papel
Copy link
Collaborator Author

Why is encyclopedia gone when i am on mobile? I disagree with this. Lobby menu is too small horizontally on desktop. Settings menu is too small horizontally on desktop. Encyclopedia menu is too small horizontally on desktop, talking about the cover card version. I'm assuming you set up the max menu default count on mobile to be 2, but I didn't test that

We had a phone call about all of these points - merging

@Jack-Papel Jack-Papel merged commit 4fe0075 into main Dec 29, 2024
2 checks passed
@Jack-Papel Jack-Papel mentioned this pull request Jan 8, 2025
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.

2 participants