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

Delete Utils.canReadGameFiles method #187

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

Conversation

Poryg1
Copy link

@Poryg1 Poryg1 commented Sep 27, 2018

What this method does is:

  1. take all already loaded scripts
  2. try to load the last one again. If success, ok, if not, error.

However, if the target script cannot be loaded, rpg_core.js cannot be loaded either. And when rpg_core.js is not loaded, Utils.canReadGameFiles cannot be executed.
Therefore this function is completely redundant, since if loading worked perfectly, we will see it, and if not, this function cannot even execute to check for it.
I also removed the code from SceneManager to make sure the game won't crash from removing the utils method.

Delete redundant Utils.canReadGameFiles. What it does is: Take already loaded scripts and try to load the last one again. However, if that script cannot be loaded, then rpg_core.js couldn't be loaded either, so the only thing this function will tell us is what we already know.
Deleted SceneManager.checkFileAccess for redundancy.
@krmbn0576
Copy link
Collaborator

Thx!
I do not think that this method is redundant because it is trying to run XMLHttpRequest correctly.
For example, if you play a game on the local system (URL starts with file:///) on Chrome, you can read the file using the <script> tag, but XMLHttpRequest is forbidden.
So, canReadGameFiles is still necessary.

But, in fact, this method is not working correctly because the forbidden XMLHttpRequest raises an error asynchronously, so try - catch statements can not catch errors.
This may have to be fixed ;-)

@Poryg1
Copy link
Author

Poryg1 commented Sep 27, 2018

Oh, I see, forgot about that one :D
Well, in that case the fix is not too difficult at least, on AudioManager it was solved via onError.bind.

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