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

Add alert for edit mode #2372

Open
96LawDawg opened this issue Nov 25, 2024 · 2 comments · May be fixed by #2383
Open

Add alert for edit mode #2372

96LawDawg opened this issue Nov 25, 2024 · 2 comments · May be fixed by #2383
Labels
enhancement New feature or request

Comments

@96LawDawg
Copy link
Collaborator

Originally suggested by @bjalder26 in #1859

It may be a good idea to provide some type of alert when any player in the room is in edit mode. The purpose would be to detect possible cheating. Maybe it doesn't matter, but we can at least have the discussion.

The way it would look is to be determined, but I would think something like changing the color of the "Edit Mode" icon/text in the side panel would be enough. I don't know if this is even technically possible without significant changes. I guess the client would send to the server some kind of notification and that would be sent out to the other clients so all of their browsers would also show the change.

@96LawDawg 96LawDawg added the enhancement New feature or request label Nov 25, 2024
@bjalder26
Copy link
Collaborator

I probably should have used the term "notification" rather than "alert", because a JavaScript alert would be a pain.

@bjalder26
Copy link
Collaborator

bjalder26 commented Nov 27, 2024

I think this would involve adjusting jeToggle() so that it sends a message toServer() when jeEnabled is changed. The message would probably need to hold the playerName of the person who toggled their JSON editor, and the state of their jeEnabled.

playerName changes would also need to send 2 messages, sending false for the old playerName, and jeEnabled for the new playerName. There would also need to be a message sent when someone leaves the game (if their JSON editor was open) - sending false for jeEnabled.

The toServer() message needs to be handled in messageReceived in server/player.mjs.

if(func == 'jeToggle')
        this.room.jeToggle(args); 

Then probably something like
this.broadcast('jeToggle', { ... })
in server/room.mjs

Then something like
onMessage('jeToggle', function() { ... })
on the client side, and that function would need to somehow (e.g. a variable) track who has their JSON editor open, and eliminate the names in that array when they close their editor (or leave, or change names). Then when the array isn't empty add a class to the editor menu icon, and when it is empty remove the class to trigger css color changes.

export function jeToggle() {
    null === jeEnabled && (jeInitTree(),
    jeAddCommands(),
    jeEmpty(),
    $("#jeText").addEventListener("input", jeColorize),
    $("#jeText").onscroll = (e => $("#jeTextHighlight").scrollTop = e.target.scrollTop)),
    jeEnabled = !jeEnabled,
    setJEenabled(jeEnabled),
    jeLoggingHTML = "",
    jeEnabled ? ($("body").classList.add("jsonEdit"),
    jeWidget && !widgets.has(jeWidget.id) && jeEmpty(),
    jeDebugViewing && (jeCallCommand(jeCommands.find(e => "je_toggleDebug" == e.id)),
    jeShowCommands())) : $("body").classList.remove("jsonEdit")
}

Also, I noticed that this seems redundant.

    jeEnabled = !jeEnabled,
    setJEenabled(jeEnabled),
function setJEenabled(v) {
jeEnabled = v;
 }

I don't really see the point of setJEenabled() if all the code to open the editor isn't in that function.

And if there is a purpose I am missing, then those two lines above should probably just be setJEenabled(!jeEnabled),.

On edit: It looks like toggleEditMode() and the edit variable were the ones to use rather than jeToggle() and jeEnabled, which are specific for the {} JSON setting in the editor.

@bjalder26 bjalder26 linked a pull request Nov 30, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants