-
Notifications
You must be signed in to change notification settings - Fork 50
Add the ability to kick controllers #678
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -688,6 +688,39 @@ func (sp *STARSPane) executeSTARSCommand(ctx *panes.Context, cmd string, tracks | |
| return | ||
| } | ||
|
|
||
| if len(cmd) >= 3 && cmd[:2] == "*K" { | ||
| // Kick controller command: *K <controller_tcp> | ||
| targetTCP := strings.TrimSpace(cmd[2:]) | ||
| if targetTCP == "" { | ||
| status.err = ErrSTARSCommandFormat | ||
| return | ||
| } | ||
|
|
||
| // Any signed-in controller can kick others (no instructor/RPO restriction) | ||
|
|
||
| // Verify the target controller exists and is signed in | ||
| if !slices.Contains(ctx.Client.State.HumanControllers, targetTCP) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a general statement, this sort of check is better to perform on the server side: since the user's view of the current state of the world in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where would this be better suited? |
||
| status.err = ErrSTARSIllegalPosition | ||
| return | ||
| } | ||
|
|
||
| // Don't allow kicking yourself | ||
| if targetTCP == ctx.UserTCP { | ||
| status.err = ErrSTARSIllegalParam | ||
| return | ||
| } | ||
|
|
||
| // Kick the controller | ||
| ctx.Client.KickController(targetTCP, func(err error) { | ||
| if err != nil { | ||
| sp.displayError(err, ctx, "") | ||
| } | ||
| }) | ||
|
|
||
| status.clear = true | ||
| return | ||
| } | ||
|
|
||
| if len(cmd) > 3 && cmd[:3] == "*F " && sp.wipSignificantPoint != nil { | ||
| if sig, ok := sp.significantPoints[cmd[3:]]; ok { | ||
| status = sp.displaySignificantPointInfo(*sp.wipSignificantPoint, sig.Location, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,17 @@ func (sd *dispatcher) SignOff(token string, _ *struct{}) error { | |
| return sd.sm.SignOff(token) | ||
| } | ||
|
|
||
| type KickControllerArgs struct { | ||
| ControllerToken string | ||
| TargetTCP string | ||
| } | ||
|
|
||
| func (sd *dispatcher) KickController(kc *KickControllerArgs, _ *struct{}) error { | ||
| defer sd.sm.lg.CatchAndReportCrash() | ||
|
|
||
| return sd.sm.KickController(kc.ControllerToken, kc.TargetTCP) | ||
| } | ||
|
|
||
| type ChangeControlPositionArgs struct { | ||
| ControllerToken string | ||
| TCP string | ||
|
|
@@ -46,7 +57,13 @@ func (sd *dispatcher) ChangeControlPosition(cs *ChangeControlPositionArgs, _ *st | |
| if ctrl, s, ok := sd.sm.LookupController(cs.ControllerToken); !ok { | ||
| return ErrNoSimForControllerToken | ||
| } else { | ||
| return s.ChangeControlPosition(ctrl.tcp, cs.TCP, cs.KeepTracks) | ||
| oldTCP := ctrl.tcp | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this related to the kick stuff? |
||
| if err := s.ChangeControlPosition(ctrl.tcp, cs.TCP, cs.KeepTracks); err != nil { | ||
| return err | ||
| } | ||
| // Update the controller mappings in SimManager after successful position change | ||
| sd.sm.UpdateControllerPosition(cs.ControllerToken, oldTCP, cs.TCP) | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -520,6 +520,72 @@ func (sm *SimManager) SignOff(token string) error { | |
| return sm.signOff(token) | ||
| } | ||
|
|
||
| func (sm *SimManager) KickController(adminToken, targetTCP string) error { | ||
| sm.mu.Lock(sm.lg) | ||
| defer sm.mu.Unlock(sm.lg) | ||
|
|
||
| // First verify that the admin has permission to kick | ||
| adminCtrl, adminSim, ok := sm.lookupController(adminToken) | ||
| if !ok { | ||
| return ErrNoSimForControllerToken | ||
| } | ||
|
|
||
| // Any signed-in controller can kick others (no instructor/RPO restriction) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this going to cause mayhem (vs. e.g. only allowing the primary controller who is presumably the one who created the sim) to do this? I'm fine with leaving it as is and seeing how it goes, though. |
||
|
|
||
| // Find the target controller in the same sim | ||
| var targetToken string | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can get rid of the for loop and if test and just do |
||
| for tcp, ctrl := range adminCtrl.asim.controllersByTCP { | ||
| if tcp == targetTCP { | ||
| targetToken = ctrl.token | ||
| break | ||
| } | ||
| } | ||
|
|
||
| if targetToken == "" { | ||
| return ErrNoControllerToKick | ||
| } | ||
|
|
||
| // Don't allow kicking yourself | ||
| if targetToken == adminToken { | ||
| return ErrCannotKickSelf | ||
| } | ||
|
|
||
| // Send immediate feedback to the admin | ||
| adminSim.PostEvent(sim.Event{ | ||
| Type: sim.StatusMessageEvent, | ||
| WrittenText: fmt.Sprintf("Kicking %s...", targetTCP), | ||
| ToController: adminCtrl.tcp, | ||
| }) | ||
|
|
||
| // Send notification to the kicked controller | ||
| adminSim.PostEvent(sim.Event{ | ||
| Type: sim.StatusMessageEvent, | ||
| WrittenText: fmt.Sprintf("You have been kicked by %s", adminCtrl.tcp), | ||
| ToController: targetTCP, | ||
| }) | ||
|
|
||
| // Perform the kick after a short delay to allow notification delivery | ||
| go func() { | ||
| time.Sleep(500 * time.Millisecond) // Give time for notification to be processed | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I worry that in some cases this may not be enough time and that in general it could be brittle. How about this for an alternate approach?
(Though having suggested that, there is some risk that someone could be unkickable if they disabled that in their local vice build. Maybe another alternative is to defer the kick until |
||
|
|
||
| sm.mu.Lock(sm.lg) | ||
| defer sm.mu.Unlock(sm.lg) | ||
|
|
||
| err := sm.signOff(targetToken) | ||
| if err == nil { | ||
| sm.lg.Infof("%s: kicked by %s", targetTCP, adminCtrl.tcp) | ||
| adminSim.PostEvent(sim.Event{ | ||
| Type: sim.StatusMessageEvent, | ||
| WrittenText: fmt.Sprintf("%s was kicked by %s", targetTCP, adminCtrl.tcp), | ||
| }) | ||
| } else { | ||
| sm.lg.Errorf("Failed to kick %s: %v", targetTCP, err) | ||
| } | ||
| }() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (sm *SimManager) signOff(token string) error { | ||
| if ctrl, s, ok := sm.lookupController(token); !ok { | ||
| return ErrNoSimForControllerToken | ||
|
|
@@ -597,6 +663,23 @@ func (sm *SimManager) lookupController(token string) (*humanController, *sim.Sim | |
| return nil, nil, false | ||
| } | ||
|
|
||
| // UpdateControllerPosition updates the controller mappings after a position change | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this part of the kick changes? |
||
| func (sm *SimManager) UpdateControllerPosition(token, oldTCP, newTCP string) { | ||
| sm.mu.Lock(sm.lg) | ||
| defer sm.mu.Unlock(sm.lg) | ||
|
|
||
| if ctrl, ok := sm.controllersByToken[token]; ok { | ||
| // Remove from old TCP mapping | ||
| delete(ctrl.asim.controllersByTCP, oldTCP) | ||
|
|
||
| // Update the controller's TCP field | ||
| ctrl.tcp = newTCP | ||
|
|
||
| // Add to new TCP mapping | ||
| ctrl.asim.controllersByTCP[newTCP] = ctrl | ||
| } | ||
| } | ||
|
|
||
| const simIdleLimit = 4 * time.Hour | ||
|
|
||
| func (sm *SimManager) SimShouldExit(sim *sim.Sim) bool { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -351,6 +351,11 @@ func (s *Sim) signOn(tcp string, instructor bool, disableTextToSpeech bool) erro | |
| s.State.Controllers[tcp] = s.SignOnPositions[tcp] | ||
| s.State.HumanControllers = append(s.State.HumanControllers, tcp) | ||
|
|
||
| // Set instructor status if the flag is true | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this related to the kick command? If not it should probably be in a separate PR. (Is there a bug this is fixing?) |
||
| if instructor { | ||
| s.Instructors[tcp] = true | ||
| } | ||
|
|
||
| if tcp == s.State.PrimaryController { | ||
| // The primary controller signed in so the sim will resume. | ||
| // Reset lastUpdateTime so that the next time Update() is | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -300,6 +300,18 @@ func uiDraw(mgr *client.ConnectionManager, config *Config, p platform.Platform, | |
| for _, event := range ui.eventsSubscription.Get() { | ||
| if event.Type == sim.ServerBroadcastMessageEvent { | ||
| uiShowModalDialog(NewModalDialogBox(&BroadcastModalDialog{Message: event.WrittenText}, p), false) | ||
| } else if event.Type == sim.StatusMessageEvent && event.ToController != "" && controlClient != nil && event.ToController == controlClient.State.UserTCP { | ||
| // Check if this is a kick notification | ||
| if strings.Contains(event.WrittenText, "You have been kicked by") { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's a little brittle to match the text here since it's non-obvious that changing it in server/manager.go would break anything. I think it would be better and more clear to add a new |
||
| client := &KickNotificationModalClient{ | ||
| message: event.WrittenText, | ||
| mgr: mgr, | ||
| config: config, | ||
| platform: p, | ||
| logger: lg, | ||
| } | ||
| uiShowModalDialog(NewModalDialogBox(client, p), true) // Show at front for visibility | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -944,6 +956,34 @@ func (e *ErrorModalClient) Draw() int { | |
| return -1 | ||
| } | ||
|
|
||
| type KickNotificationModalClient struct { | ||
| message string | ||
| mgr *client.ConnectionManager | ||
| config *Config | ||
| platform platform.Platform | ||
| logger *log.Logger | ||
| } | ||
|
|
||
| func (k *KickNotificationModalClient) Title() string { return "Kicked from Simulation" } | ||
| func (k *KickNotificationModalClient) Opening() {} | ||
|
|
||
| func (k *KickNotificationModalClient) Buttons() []ModalDialogButton { | ||
| return []ModalDialogButton{{ | ||
| text: "Ok", | ||
| action: func() bool { | ||
| // When the user clicks OK, show the connection dialog | ||
| uiShowConnectDialog(k.mgr, true, k.config, k.platform, k.logger) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, is this necessary? I'd have guessed that after they were kicked the code that checks if you're connected to a sim would notice they weren't and would put this up on its own. (Then that'd be nice so you wouldn't have to carry along all of the extra members in the |
||
| return true | ||
| }, | ||
| }} | ||
| } | ||
|
|
||
| func (k *KickNotificationModalClient) Draw() int { | ||
| text, _ := util.WrapText(k.message, 80, 0, true) | ||
| imgui.Text("\n\n" + text + "\n\n") | ||
| return -1 | ||
| } | ||
|
|
||
| func ShowErrorDialog(p platform.Platform, lg *log.Logger, s string, args ...interface{}) { | ||
| d := NewModalDialogBox(&ErrorModalClient{message: fmt.Sprintf(s, args...)}, p) | ||
| uiShowModalDialog(d, true) | ||
|
|
||
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.
Could you switch
*Kto be.KICK? I'd like to keep the stuff that aren't real-world STARS commands to all start with..