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

Folder names to settings #324

Open
wants to merge 2 commits into
base: xml_converter
Choose a base branch
from

Conversation

klingbolt
Copy link
Contributor

Moves some variable to the Settings folder.

Dependent on #323

Comment on lines 5 to +6
func _ready():
#TODO: Move these to global file Settings so they can be customized
self.user_data_dir = str(OS.get_user_data_dir())
self.downloaded_markers_dir = self.user_data_dir.plus_file("packs")
self.unsaved_markers_dir = self.user_data_dir.plus_file("protobins")
FileHandler.create_directory_if_missing(self.downloaded_markers_dir)
FileHandler.create_directory_if_missing(self.unsaved_markers_dir)
pass
Copy link
Owner

Choose a reason for hiding this comment

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

This function can be deleted if it only contained the no-op pass

Comment on lines +69 to +70
FileHandler.create_directory_if_missing(self.downloaded_markers_dir)
FileHandler.create_directory_if_missing(self.unsaved_markers_dir)
Copy link
Owner

Choose a reason for hiding this comment

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

What happens if self.downloaded_markers_dir or self.unsaved_markers_dir changes during runtime?

Comment on lines +83 to +85
#"user_data_dir" : user_data_dir,
#"downloaded_markers_dir": downloaded_markers_dir,
#"unsaved_markers_dir": unsaved_markers_dir,
Copy link
Owner

Choose a reason for hiding this comment

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

Why are we loading this data but not saving it? Won't this effectively delete "user_data_dir", "download_markers_dir", and "unsaved_markers_dir" from the settings every time the data is saved?

Additionally, do you want to save this data to the dynamic default values of OS.get_user_data_dir() if the user has not explicitly configured a custom path?

Spatial.gd Outdated
@@ -647,7 +648,7 @@ func save_current_map_data():
var file = File.new()
file.open(self.marker_file_path, file.WRITE)
file.store_buffer(packed_bytes)
set_unsaved_changes(false)
Copy link
Owner

Choose a reason for hiding this comment

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

Why are you removing "marking that there are no longer unsaved changes" from the function that saves changes and therefore removes unsaved changes?

Comment on lines +85 to +86
if Settings.unsaved_changes:
set_unsaved_changes(Settings.unsaved_changes)
Copy link
Owner

Choose a reason for hiding this comment

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

You probably dont need a local shadow of Settings.unsaved_changes here. Otherwise you have to keep them in sync, which it does not look like you are doing in this pr.

Spatial.gd Outdated
@@ -767,7 +768,7 @@ func set_icon_position(new_position: Vector3, waypoint_icon: Waypoint.Icon, icon
position.set_y(new_position.y)
position.set_z(new_position.z)
icon.set_position(new_position)
set_unsaved_changes(true)
Settings.set_unsaved_changes(true)
Copy link
Owner

Choose a reason for hiding this comment

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

Does this function exist? Did you test this code?

Spatial.gd Outdated
@@ -853,18 +854,19 @@ func new_trail_point_after(waypoint_trail: Waypoint.Trail, trail3d: Spatial, tra

func refresh_trail3d_points(trail3d: Spatial):
trail3d.refresh_mesh()
set_unsaved_changes(true)
Settings.set_unsaved_changes(true)
Copy link
Owner

Choose a reason for hiding this comment

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

Does this function exist?

Spatial.gd Outdated

func refresh_trail2d_points(trail2d: Line2D):
trail2d.refresh_points()
set_unsaved_changes(true)
Settings.set_unsaved_changes(true)
Copy link
Owner

Choose a reason for hiding this comment

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

Does this function exist?

Spatial.gd Outdated


################################################################################
# Signal Functions
################################################################################
func _on_SaveTrail_pressed():
save_current_map_data()
Settings.set_unsaved_changes(false)
Copy link
Owner

Choose a reason for hiding this comment

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

Does this function exist?

Comment on lines +23 to +25
#We save the marker data in this directory when the files are have been split
#by Map ID. All changes made by the editor are automatically saved in these
#files prior to export.
Copy link
Owner

Choose a reason for hiding this comment

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

nit: same as in #323 where this comment is from I think. Add a space after each #.

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

Successfully merging this pull request may close these issues.

2 participants