🚀 Add timespan details expression & Improvements #4661

AyhamAl-Ali · 2022-03-12T23:49:47Z

Description

Added a new expression to get timespan information such as days/minutes/etc.
Supported weeks, months and years in toString of timespan which makes this code broadcast 365 days output as 1 year instead of 365 days
Overall code improvements
Supported days in DD:HH:MM:SS[.ms] format
Improved toString method

NOTE: This is a breaking change (probably) since some users expect the output to be in days and parsing it as that however the output now supports years, months and weeks therefore this should be either merged in a major update or keep the old behavior of toString

Target Minecraft Versions: Any
Requirements: None
Related Issues: None

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java

src/main/java/ch/njol/skript/util/Timespan.java

AyhamAl-Ali · 2022-03-15T12:11:18Z

I don't much like the usage of indices here so if you have a better and clean way let me know

Skript/src/main/java/ch/njol/skript/util/Timespan.java

Lines 51 to 57 in 1669cfa

    
           new NonNullPair<>(names[7], times[7]), 
        
           new NonNullPair<>(names[6], times[6]), 
        
           new NonNullPair<>(names[5], times[5]), 
        
           new NonNullPair<>(names[4], times[4]), 
        
           new NonNullPair<>(names[3], times[3]), 
        
           new NonNullPair<>(names[2], times[2]), 
        
           new NonNullPair<>(names[1], times[1])

Same goes for the new getters

Pikachu920 · 2022-03-18T01:58:59Z

I don't much like the usage of indices here so if you have a better and clean way let me know

Skript/src/main/java/ch/njol/skript/util/Timespan.java

Lines 51 to 57 in 1669cfa

new NonNullPair<>(names[7], times[7]),

new NonNullPair<>(names[6], times[6]),

new NonNullPair<>(names[5], times[5]),

new NonNullPair<>(names[4], times[4]),

new NonNullPair<>(names[3], times[3]),

new NonNullPair<>(names[2], times[2]),

new NonNullPair<>(names[1], times[1])

Same goes for the new getters

it's probably best replaced by an enum

Pikachu920 · 2022-03-18T01:58:59Z

I don't much like the usage of indices here so if you have a better and clean way let me know

Skript/src/main/java/ch/njol/skript/util/Timespan.java

Lines 51 to 57 in 1669cfa

new NonNullPair<>(names[7], times[7]),

new NonNullPair<>(names[6], times[6]),

new NonNullPair<>(names[5], times[5]),

new NonNullPair<>(names[4], times[4]),

new NonNullPair<>(names[3], times[3]),

new NonNullPair<>(names[2], times[2]),

new NonNullPair<>(names[1], times[1])

Same goes for the new getters

it's probably best replaced by an enum

src/main/java/ch/njol/skript/util/Timespan.java

TPGamesNL · 2022-07-09T10:47:44Z

src/main/java/ch/njol/skript/util/Timespan.java

 		long t = 0;
 		boolean minecraftTime = false;
 		boolean isMinecraftTimeSet = false;
-		if (s.matches("^\\d+:\\d\\d(:\\d\\d)?(\\.\\d{1,4})?$")) { // MM:SS[.ms] or HH:MM:SS[.ms]
+
+		if (TIMESPAN_PATTERN.matcher(s).matches()) { // MM:SS[.ms] or HH:MM:SS[.ms] or DD:HH:MM:SS[.ms]
 			final String[] ss = s.split("[:.]");


Wouldn't it be better to use regex groups for this instead? Would probably simplify the code below quite a bit

Sounds a little complex and out-of-scope, maybe in another PR

src/main/java/ch/njol/skript/util/Timespan.java

Co-authored-by: TPGamesNL <29547183+TPGamesNL@users.noreply.github.com>

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java

src/main/java/ch/njol/skript/util/Timespan.java

… ench/timespan-improve

src/main/java/ch/njol/skript/util/Timespan.java

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java

… ench/timespan-improve

away

APickledWalrus

None of these are necessarily required, just think they're worth discussing :)

src/main/java/ch/njol/skript/util/Timespan.java

APickledWalrus · 2023-08-05T01:23:39Z

src/main/java/ch/njol/skript/util/Timespan.java

 	}
-	
+
 	public long getMilliSeconds() {


I wonder if it's worth adding a default TimePeriod.MILLISECOND option

Not sure about that, It's just a 1L which doesn't have much use in math/comparison otherwise let me know :)

src/main/java/ch/njol/skript/util/Timespan.java

…improve

APickledWalrus

Good work!

I do still think it would be good to support milliseconds as a TimePeriod (also goes with my other comment about an additional constructor). With that, we would be best off to replace getMilliSeconds() and getTicks() with a more generic getAs(TimePeriod) method (name TBD)

src/main/java/ch/njol/skript/util/Timespan.java

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java

Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>

AyhamAl-Ali · 2024-01-21T23:09:16Z

@APickledWalrus I have added the suggested constructor and getAs(TimePeriod), as well as deprecating the old methods however, I didn't add the Milliseconds support in TimePeriod as that would break the current Timespan parsing and toString output which should have ticks as the least value since that's what Minecraft support.

🚀 Add timespan details expression

870e8ff

AyhamAl-Ali added enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature Pull request adding a new feature. labels Mar 12, 2022

Merge branch 'master' into ench/timespan-improve

5f7f92e

Pikachu920 requested changes Mar 15, 2022

View reviewed changes

TPGamesNL reviewed Mar 15, 2022

View reviewed changes

src/main/java/ch/njol/skript/util/Timespan.java Outdated Show resolved Hide resolved

Changes

1669cfa

AyhamAl-Ali requested review from Pikachu920 and TPGamesNL March 15, 2022 12:11

Improvements

8fe9d0a

Pikachu920 requested changes Mar 18, 2022

View reviewed changes

src/main/java/ch/njol/skript/util/Timespan.java Outdated Show resolved Hide resolved

Changes

9367856

AyhamAl-Ali requested a review from Pikachu920 March 20, 2022 00:04

Add DD:HH:MM:SS.ms + improve toString

cf00390

AyhamAl-Ali added the breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) label Jun 11, 2022

Merge branch 'master' into ench/timespan-improve

68a2096

TPGamesNL previously requested changes Jul 9, 2022

View reviewed changes

AyhamAl-Ali and others added 5 commits July 9, 2022 05:16

Update src/main/java/ch/njol/skript/util/Timespan.java

8af2008

Co-authored-by: TPGamesNL <29547183+TPGamesNL@users.noreply.github.com>

Update src/main/java/ch/njol/skript/util/Timespan.java

fdd2c9c

Co-authored-by: TPGamesNL <29547183+TPGamesNL@users.noreply.github.com>

Update src/main/java/ch/njol/skript/util/Timespan.java

bf52019

Co-authored-by: TPGamesNL <29547183+TPGamesNL@users.noreply.github.com>

Update src/main/java/ch/njol/skript/util/Timespan.java

3a4d6c3

Co-authored-by: TPGamesNL <29547183+TPGamesNL@users.noreply.github.com>

Testing

a32ca2a

APickledWalrus requested changes Jul 15, 2022

View reviewed changes

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java Outdated Show resolved Hide resolved

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java Outdated Show resolved Hide resolved

src/main/java/ch/njol/skript/util/Timespan.java Outdated Show resolved Hide resolved

Improvements - Unfinished code

6372da2

AyhamAl-Ali marked this pull request as draft July 16, 2022 01:45

AyhamAl-Ali added 2 commits January 1, 2023 22:37

Update enums

0ec45dd

Merge remote-tracking branch 'AyhamAl-Ali/ench/timespan-improve' into…

9f4bbfb

… ench/timespan-improve

Pikachu920 reviewed Apr 19, 2023

View reviewed changes

src/main/java/ch/njol/skript/util/Timespan.java Outdated Show resolved Hide resolved

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java Outdated Show resolved Hide resolved

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java Outdated Show resolved Hide resolved

TheLimeGlass approved these changes Apr 19, 2023

View reviewed changes

AyhamAl-Ali added 2 commits April 20, 2023 03:24

Pikachu's quality requested changes 🚀

201b546

Merge remote-tracking branch 'AyhamAl-Ali/ench/timespan-improve' into…

37afcb9

… ench/timespan-improve

AyhamAl-Ali requested a review from Pikachu920 April 20, 2023 00:24

AyhamAl-Ali changed the title ~~🚀 Add timespan details expression~~ 🚀 Add timespan details expression & Improvements May 15, 2023

Pikachu920 approved these changes May 16, 2023

View reviewed changes

kiip1 approved these changes May 18, 2023

View reviewed changes

Merge branch 'master' into ench/timespan-improve

0283bea

TheLimeGlass removed the request for review from TPGamesNL August 5, 2023 00:48

APickledWalrus requested changes Aug 5, 2023

View reviewed changes

Moderocky force-pushed the master branch 3 times, most recently from bd134d0 to 3f08853 Compare September 16, 2023 16:59

Moderocky changed the base branch from master to dev/feature September 18, 2023 09:15

AyhamAl-Ali added 2 commits October 6, 2023 01:28

Merge remote-tracking branch 'origin/dev/feature' into ench/timespan-…

2160bdb

…improve

Apply suggestions

63b281f

AyhamAl-Ali requested a review from APickledWalrus October 5, 2023 23:06

typo

a2be2b1

APickledWalrus approved these changes Dec 15, 2023

View reviewed changes

src/main/java/ch/njol/skript/util/Timespan.java Show resolved Hide resolved

src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java Outdated Show resolved Hide resolved

AyhamAl-Ali and others added 4 commits December 15, 2023 13:04

Update src/main/java/ch/njol/skript/expressions/ExprTimespanDetails.java

6be6300

Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>

Merge branch 'dev/feature' into ench/timespan-improve

2ac1b90

Add Timespan(TimePeriod, long) and deprecate fromTicks

2b18a04

Add getAs(TimePeriod)

1b845c7

AyhamAl-Ali added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jan 21, 2024

sovdeeth added the 2.9 Targeting a 2.9.X version release label Jan 29, 2024

Merge branch 'dev/feature' into ench/timespan-improve

001c9e3

sovdeeth merged commit c59ff5b into SkriptLang:dev/feature Mar 19, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚀 Add timespan details expression & Improvements #4661

🚀 Add timespan details expression & Improvements #4661

AyhamAl-Ali commented Mar 12, 2022 •

edited

Loading

AyhamAl-Ali commented Mar 15, 2022

Pikachu920 commented Mar 18, 2022

Pikachu920 commented Mar 18, 2022

TPGamesNL Jul 9, 2022

AyhamAl-Ali Jan 1, 2023

APickledWalrus left a comment

APickledWalrus Aug 5, 2023

AyhamAl-Ali Oct 5, 2023

APickledWalrus left a comment

AyhamAl-Ali commented Jan 21, 2024

🚀 Add timespan details expression & Improvements #4661

🚀 Add timespan details expression & Improvements #4661

Conversation

AyhamAl-Ali commented Mar 12, 2022 • edited Loading

Description

AyhamAl-Ali commented Mar 15, 2022

Pikachu920 commented Mar 18, 2022

Pikachu920 commented Mar 18, 2022

TPGamesNL Jul 9, 2022

Choose a reason for hiding this comment

AyhamAl-Ali Jan 1, 2023

Choose a reason for hiding this comment

APickledWalrus left a comment

Choose a reason for hiding this comment

APickledWalrus Aug 5, 2023

Choose a reason for hiding this comment

AyhamAl-Ali Oct 5, 2023

Choose a reason for hiding this comment

APickledWalrus left a comment

Choose a reason for hiding this comment

AyhamAl-Ali commented Jan 21, 2024

AyhamAl-Ali commented Mar 12, 2022 •

edited

Loading