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

Remove the extra slash while loading resources. #893

Merged
8 commits merged into from
May 16, 2022

Conversation

line-bear
Copy link
Contributor

Description

The new default config file already contains / at the end of path string, which may cause double slash while loading resources, It seems to work fine at the moment, but it is recommended to remove it to prevent unpredictable situations.

Issues fixed by this PR

Type of changes

  • Bug fix
  • New feature
  • Enhancement
  • Documentation

Checklist:

  • My code follows the style guidelines of this project
  • My pull request is unique and no other pull requests have been opened for these changes
  • I have read the Contributing note and Code of conduct
  • I am responsible for any copyright issues with my code if it occurs in the future.

@ghost
Copy link

ghost commented May 14, 2022

We can not be sure that someone editing config file will remember to add / at the end of string.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Let's use proper path joining.

import java.nio.file.Paths;

src/main/java/emu/grasscutter/Configuration.java Outdated Show resolved Hide resolved
src/main/java/emu/grasscutter/Configuration.java Outdated Show resolved Hide resolved
src/main/java/emu/grasscutter/Configuration.java Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented May 14, 2022

You should also perform same changes in all other files where *_FOLDER are used. Most likely we should mark them as private and instead provide helper functions like DATA for all other ones that don't have one currently.

line-bear and others added 3 commits May 15, 2022 15:34
Co-authored-by: HotaruYS <105128850+HotaruYS@users.noreply.github.com>
Co-authored-by: HotaruYS <105128850+HotaruYS@users.noreply.github.com>
Co-authored-by: HotaruYS <105128850+HotaruYS@users.noreply.github.com>
Copy link
Contributor Author

@line-bear line-bear left a comment

Choose a reason for hiding this comment

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

Use path join to prevent double slash

@line-bear
Copy link
Contributor Author

You should also perform same changes in all other files where *_FOLDER are used. Most likely we should mark them as private and instead provide helper functions like DATA for all other ones that don't have one currently.

all done, have fun

Copy link
Contributor Author

@line-bear line-bear left a comment

Choose a reason for hiding this comment

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

Mark fields as private to prevent use *FOLDER directly

@ghost
Copy link

ghost commented May 15, 2022

Thanks. Looks good to me now.

I'll most likely go around whole codebase in another PR later and see if we can refactor Utils.toFilePath to be a wrapper for path join instead of dumb replacing / with separator. I've also spotted few places that forget to use Utils.toFilePath so will need to handle that too.

@line-bear
Copy link
Contributor Author

Thanks. Looks good to me now.

I'll most likely go around whole codebase in another PR later and see if we can refactor Utils.toFilePath to be a wrapper for path join instead of dumb replacing / with separator. I've also spotted few places that forget to use Utils.toFilePath so will need to handle that too.

Waitting for your message :)

@ghost ghost merged commit 58df221 into Grasscutters:development May 16, 2022
@exzork exzork mentioned this pull request May 16, 2022
8 tasks
@line-bear line-bear deleted the development branch May 18, 2022 20:58
@line-bear line-bear restored the development branch May 18, 2022 21:03
@line-bear line-bear deleted the development branch May 18, 2022 21:04
Birdulon pushed a commit to Birdulon/Grasscutter that referenced this pull request Aug 21, 2022
* Remove the extra slash

* Update src/main/java/emu/grasscutter/Configuration.java

Co-authored-by: HotaruYS <105128850+HotaruYS@users.noreply.github.com>

* Update src/main/java/emu/grasscutter/Configuration.java

Co-authored-by: HotaruYS <105128850+HotaruYS@users.noreply.github.com>

* Update src/main/java/emu/grasscutter/Configuration.java

Co-authored-by: HotaruYS <105128850+HotaruYS@users.noreply.github.com>

* Import java.nio.file.Paths to use Paths.get

* Mark fields as private to prevent use *FOLDER directly

* Remove unnecessary slash

Co-authored-by: HotaruYS <105128850+HotaruYS@users.noreply.github.com>
This pull request was closed.
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.

None yet

1 participant