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.

Sign up for GitHub

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

refactor: moving time methods to ModLoaderTime #346

Open
wants to merge 6 commits into
base: development
Choose a base branch
from
Open

refactor: moving time methods to ModLoaderTime #346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
53d7ca5
Spliting Date & Time Methods in to Dedicated Class
AlajeBash Oct 19, 2023
76505f1
Update addons/mod_loader/api/time.gd
AlajeBash Oct 19, 2023
364600c
Update addons/mod_loader/api/time.gd
AlajeBash Oct 19, 2023
c66631d
Update addons/mod_loader/api/time.gd
AlajeBash Oct 19, 2023
4f82dd9
resolved the suggestions
AlajeBash Oct 19, 2023
a73641f
Update addons/mod_loader/api/time.gd
AlajeBash Oct 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions addons/mod_loader/api/log.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,21 +446,6 @@ class ModLoaderLogCompare:
# Internal Date Time
# =============================================================================

# Returns the current time as a string in the format hh:mm:ss
static func _get_time_string() -> String:
var date_time := Time.get_datetime_dict_from_system()
return "%02d:%02d:%02d" % [ date_time.hour, date_time.minute, date_time.second ]


# Returns the current date as a string in the format yyyy-mm-dd
static func _get_date_string() -> String:
var date_time := Time.get_datetime_dict_from_system()
return "%s-%02d-%02d" % [ date_time.year, date_time.month, date_time.day ]


# Returns the current date and time as a string in the format yyyy-mm-dd_hh:mm:ss
static func _get_date_time_string() -> String:
return "%s_%s" % [ _get_date_string(), _get_time_string() ]



Expand Down Expand Up @@ -505,7 +490,7 @@ static func _rotate_log_file() -> void:
var error := log_file.open(MOD_LOG_PATH, File.WRITE)
if not error == OK:
assert(false, "Could not open log file, error code: %s" % error)
log_file.store_string('%s Created log' % _get_date_string())
log_file.store_string('%s Created log' % ModLoaderTime.get_date_string())
Copy link
Member

Choose a reason for hiding this comment

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

Please search the entire codebase for calls to the Time utility functions and update them to ModLoaderTime. In Godot, you can press Ctrl + Alt + F to search the entire project. Additionally, please perform a test run. You can create an empty Godot project and symlink the repository into the addons folder for testing.

Copy link
Author

Choose a reason for hiding this comment

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

@KANAjetzt is there need for Internal Date Time functions in addons/mod_loader/setup/setup_log.gd ?

Copy link
Member

@KANAjetzt KANAjetzt Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You can keep everything related to the setup (everything in the setup directory) as it is. The setup has to be completely separate, which is why there is a lot of duplicated code in these files.

Copy link
Author

Choose a reason for hiding this comment

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

Ok thank you

Copy link
Member

Choose a reason for hiding this comment

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

There is one more call to _get_date_time_string()

godot-mod-loader/addons/mod_loader/api/log.gd

Line 479 in a73641f

var datetime := _get_date_time_string().replace(":", ".")

log_file.close()


Expand Down
19 changes: 19 additions & 0 deletions addons/mod_loader/api/time.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# This class provides utility functions to retrieve the current time, date, and date-time in specific string formats.
class_name ModLoaderTime
extends Node

Copy link
Member

@KANAjetzt KANAjetzt Oct 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

One empty line missing.
If you are interested you can read up on the GDScript style guide here.

Copy link
Member

Choose a reason for hiding this comment

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

The empty line is still missing.

# Returns the current time as a string in the format hh:mm:ss
static func get_time_string() -> String:
var date_time := Time.get_datetime_dict_from_system()
return "%02d:%02d:%02d" % [ date_time.hour, date_time.minute, date_time.second ]


# Returns the current date as a string in the format yyyy-mm-dd
static func get_date_string() -> String:
var date_time := Time.get_datetime_dict_from_system()
return "%s-%02d-%02d" % [ date_time.year, date_time.month, date_time.day ]


# Returns the current date and time as a string in the format yyyy-mm-dd_hh:mm:ss
static func get_date_time_string() -> String:
return "%s_%s" % [ _get_date_string(), _get_time_string() ]