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

EvtRealTime #7332

Open
wants to merge 17 commits into
base: dev/feature
Choose a base branch
from

Conversation

TheAbsolutionism
Copy link
Contributor

@TheAbsolutionism TheAbsolutionism commented Dec 30, 2024

Description

This PR aims to add an event that is called when the local time of the system reaches the provided time(s)
Allowing users to make real time events for their server.


Target Minecraft Versions: any
Requirements: none
Related Issues: #3378

Copy link
Member

@Efnilite Efnilite left a comment

Choose a reason for hiding this comment

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

perhaps some tests may be warranted

src/main/java/ch/njol/skript/events/EvtServerTime.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/events/EvtServerTime.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/util/Time.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/util/Time.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/util/Time.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/events/EvtServerTime.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/events/EvtServerTime.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/events/EvtServerTime.java Outdated Show resolved Hide resolved
@Efnilite Efnilite added the feature Pull request adding a new feature. label Dec 31, 2024
@TheAbsolutionism TheAbsolutionism changed the title EvtServerTime EvtSystemTime Dec 31, 2024
@Burbulinis
Copy link

What do you think of the syntax pattern at %time% [in] real time instead of the current on real time (of|at) %time%? The latter doesn't sound grammatically correct or natural.

@Moderocky
Copy link
Member

Yes

@TheAbsolutionism
Copy link
Contributor Author

What do you think of the syntax pattern at %time% [in] real time instead of the current on real time (of|at) %time%? The latter doesn't sound grammatically correct or natural.

Well, imo, it would make it look silly if users provided multiple times. on 3pm, 6pm, 9pm in real time:
But it's whatever the mass majority chooses on.

@Burbulinis
Copy link

Burbulinis commented Jan 2, 2025

What do you think of the syntax pattern at %time% [in] real time instead of the current on real time (of|at) %time%? The latter doesn't sound grammatically correct or natural.

Well, imo, it would make it look silly if users provided multiple times. on 3pm, 6pm, 9pm in real time: But it's whatever the mass majority chooses on.

It won’t be on …, but at … like the current EvtWorldTime (or however the class is named). And personally, it sounds fine

@TheAbsolutionism
Copy link
Contributor Author

It won’t be on …, but at … like the current EvtWorldTime (or however the class is named)

Right, but regardless, to me it would still look silly. at 3pm, 6pm, 9pm in real time:

@TheAbsolutionism
Copy link
Contributor Author

Again, it's whatever the majority chooses. Can't fight that

src/main/java/ch/njol/skript/util/Time.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/events/EvtSystemTime.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/events/EvtSystemTime.java Outdated Show resolved Hide resolved
@sovdeeth
Copy link
Member

sovdeeth commented Jan 3, 2025

It won’t be on …, but at … like the current EvtWorldTime (or however the class is named)

Right, but regardless, to me it would still look silly. at 3pm, 6pm, 9pm in real time:

seems fine to me, since proper use would entail using and
at 3 pm, 6 pm, and 9 pm in real time

on real time of 3 pm, 6 pm, and 9 pm makes a lot less sense to me

@TheAbsolutionism
Copy link
Contributor Author

Well, that seems to be the majority then. I will make change

@TheAbsolutionism TheAbsolutionism changed the title EvtSystemTime EvtRealTime Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull request adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants