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

Jump to bottom

Locally Hosted Configuration has CORS conflict with API #1078

Closed
athias opened this issue May 18, 2024 · 9 comments · Fixed by #1122
Closed

Locally Hosted Configuration has CORS conflict with API #1078

athias opened this issue May 18, 2024 · 9 comments · Fixed by #1122
Labels
Bug Something isn't working

Comments

@athias
Copy link

athias commented May 18, 2024

Locally Hosted in this context refers to running from source, using npm run start:dev

It's been reported already in discord that the game fails to run the GameOverPhase, although the source of the issue was unknown. I ran into this issue and identified the source/conflict preventing it from loading properly.

Essentially the game is attempting to run the api.pokerogue.net, specifically: Access to fetch at 'https://api.pokerogue.net/savedata/newclear?slot=0'

The source of this is caused when the individual connects via the Network option instead of localhost. This causes the utils.ts to not recognize it as being run locally, specifically localhost

To resolve this issue locally I modified the src/utils.ts line 218 to include my hosted IP address in the secondary option of the window.location.hostname

This example uses 10.10.10.10 as my hosted IP

export const sessionIdKey = 'pokerogue_sessionId';
export const isLocal = window.location.hostname === 'localhost' || window.location.hostname === '10.10.10.10';
export const serverUrl = isLocal ? 'http://localhost:8001' : 'http://10.10.10.10:8001';
export const apiUrl = isLocal ? serverUrl : 'https://api.pokerogue.net';
@Madmadness65 Madmadness65 added the Bug Something isn't working label May 18, 2024
@IceFire03
Copy link
Contributor

Would it make more sense to check if the hostname != pokerogue and set the serverUrl based on the hostname, then?

@athias
Copy link
Author

athias commented May 18, 2024

Initially I tried to add in some overrides to define locally hosted server configs and have utils.ts simply update accordingly. Unfortunately I probably don't know enough JS to get it working properly because I ran into some clear formatting issue.

@CodeTappert
Copy link
Contributor

Wouldnt

// Check if the current hostname is 'localhost' or an IP address, and ensure a port is specified
export const isLocal = (window.location.hostname === 'localhost' || 
    /^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$/.test(window.location.hostname)) && 
    window.location.port !== '';

// Set the server URL based on whether it's local or not
export const serverUrl = isLocal ? `${window.location.hostname}:${window.location.port}` : '';

not be enough?

So it is local if run on localhost or some IP with a port no matter what. And this should also fix the serverURL as far as i know

@IceFire03
Copy link
Contributor

Yeah that makes sense to me

@athias
Copy link
Author

athias commented May 18, 2024

Wouldnt

// Check if the current hostname is 'localhost' or an IP address, and ensure a port is specified
export const isLocal = (window.location.hostname === 'localhost' || 
    /^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$/.test(window.location.hostname)) && 
    window.location.port !== '';

// Set the server URL based on whether it's local or not
export const serverUrl = isLocal ? `${window.location.hostname}:${window.location.port}` : '';

not be enough?

So it is local if run on localhost or some IP with a port no matter what. And this should also fix the serverURL as far as i know

I tested this fix on my setup and had no issues using the network address and completing the area where it previously failed:

Screenshot from 2024-05-18 16-20-35

@CodeTappert
Copy link
Contributor

Then please create a Pull Request with this change!
(And maybe give some credit to me by linking my comment with the suggestion in it)

@IceFire03
Copy link
Contributor

@CodeTappert I think it would be best for you to make the pull request and link it to this issue seeing as it's your code :)

@CodeTappert
Copy link
Contributor

Well then i will do that in like 9h or so. After i slept

@CodeTappert CodeTappert mentioned this issue May 19, 2024
Merged
@CodeTappert
Copy link
Contributor

@IceFire03 See #1122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Changed How is local is checked and how serverURL is build to prevent… CodeTappert/pokerogue
4 participants
@athias @CodeTappert @Madmadness65 @IceFire03