Skip to content

Modernized start, stop and restart commands #4023

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Nico8340
Copy link
Member

@Nico8340 Nico8340 commented Feb 13, 2025

Modernized start, stop, restart commands which now also support directories, see the examples below, and from now on, the restart command not just restarts the already running resources specified as parameters, but also starts the stopped ones.
Closes #2549.

start play
start admin ip2c playercolors
start [web]

stop play
stop admin ip2c playercolors
stop [web]

restart play
restart admin ip2c playercolors
restart [web]

@Lpsd
Copy link
Member

Lpsd commented Feb 14, 2025

We should probably add an argument ensure to restartResource (defaulting to false)?

The reason I don't suggest adding a separate API function for this (e.g: ensureResource), is because we'd need to repeat the arguments and associated logic for those anyway.

Edit: since you'd need to upgrade restartResource to the new arg parser anyway, the code would actually end up being reasonably sized; having an additional function such as ensureResource wouldn't be so bad.

So, it's up to you and whatever others think is best.

@TheNormalnij
Copy link
Member

We should probably add an argument ensure to restartResource (defaulting to false)?

Why do you need that when you can get the state of your resource before calling? A new argument doesn't improve readability

@TheNormalnij
Copy link
Member

I suggest to remove a new 'ensure' command and just change 'restart' behavior. Current 'restart' behavior is disappointing.

@Lpsd
Copy link
Member

Lpsd commented Feb 14, 2025

I suggest to remove a new 'ensure' command and just change 'restart' behavior. Current 'restart' behavior is disappointing.

I agree with this too, but would we want to change the behaviour of restartResource function by default?

Technically that isn't backwards compatible, someone may rely on the fact a resource must be running when this function or command is called.

Nonetheless, it would be much better behaviour for the restart command and function to start resources that aren't running, instead of doing nothing/returning false.

@Fernando-A-Rocha
Copy link
Contributor

Changing restart command functionality to support starting a resource even if it is stopped does not make sense, as the name implies a stop then a start, not just a start.

New ensure command makes sense, it's practical. I support it.

@Nico8340
Copy link
Member Author

We should probably add an argument ensure to restartResource (defaulting to false)?

The reason I don't suggest adding a separate API function for this (e.g: ensureResource), is because we'd need to repeat the arguments and associated logic for those anyway.

Edit: since you'd need to upgrade restartResource to the new arg parser anyway, the code would actually end up being reasonably sized; having an additional function such as ensureResource wouldn't be so bad.

So, it's up to you and whatever others think is best.

I guess we can discuss that later, but in a separate issue or pull request, this one is focused solely on improving the commands

@Nico8340
Copy link
Member Author

Changing restart command functionality to support starting a resource even if it is stopped does not make sense, as the name implies a stop then a start, not just a start.

New ensure command makes sense, it's practical. I support it.

I want to introduce the new command because restart is well known and widely used, and I don't want to change that. ensure is also common in other game engine like multiplayer modifications, so it should feel familiar to developers interested in modern stuff

@TheNormalnij
Copy link
Member

I want to introduce the new command because restart is well known and widely used, and I don't want to change that. ensure is also common in other game engine like multiplayer modifications, so it should feel familiar to developers interested in modern stuff

As far as I know, only FiveM has the ensure command. The suggested restart behavior is much more common in Linux, e.g. the service command works in this way. I expect that the restart command starts stopped services and resources.
I can't imagine that somebody wouldn't expect to start a resource using the restart command.

I see no reason to have two commands with almost identical behavior instead of using one command with expected name.

Technically that isn't backwards compatible, someone may rely on the fact a resource must be running when this function or command is called.

I can't imagine a scenario when you need that. I would suggest writing a function that implements the old behavior if someone needs that. A failed restart commad forced me to use start in all my cases.

@TheNormalnij TheNormalnij added the feedback Further information is requested label Mar 2, 2025
@blow1d
Copy link

blow1d commented May 8, 2025

Up

Copy link
Contributor

@Fernando-A-Rocha Fernando-A-Rocha left a comment

Choose a reason for hiding this comment

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

Perfect, gj

@blow1d
Copy link

blow1d commented Jun 1, 2025

This should have been merged a long time ago, pr dessa is very economical in some factors

@Nico8340
Copy link
Member Author

I'm okay with renaming the ensure command to restart if it matters in merging the pull request.

@MohabCodeX
Copy link
Contributor

Good job 🥳🤙🏻
I think it might be better to integrate the behavior of ensure into the existing restart command.
Users who want the behavior of ensure will likely use restart instinctively anyway.
If a script isn't running, and someone uses restart, it's usually because they want it to start so letting restart handle both running and stopped resources could reduce confusion and eliminate the need to introduce a new command

@Kinimel
Copy link

Kinimel commented Jul 31, 2025

GG Good job

@Nico8340 Nico8340 changed the title Modernized start, stop, restart and new ensure command Modernized start, stop and restart commands Jul 31, 2025
@Nico8340
Copy link
Member Author

I'm okay with renaming the ensure command to restart if it matters in merging the pull request.

Done. Now, the restart command is more Linux like.

Copy link
Contributor

@Fernando-A-Rocha Fernando-A-Rocha left a comment

Choose a reason for hiding this comment

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

Very well.
This will benefit everyone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add resource server commands
7 participants