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

Add properties to App Container #76

Merged
merged 3 commits into from
Aug 21, 2018

Conversation

cbaconnier
Copy link
Contributor

(int)$app->required_age; About this one, Steam seems to returns only 0 or "18" I choose to cast it to int keep some consistency.

@stygiansabyss
Copy link
Contributor

I will be happy to merge this in. Could you update the example response for this with how the new fields appear?

Thanks!

@stygiansabyss
Copy link
Contributor

I am slightly concerned by the properties with no data. Primarily appId, but others as well (isFree should show something no matter what).

Do you know if its just an error of the output or if it is actually empty/null?

@cbaconnier
Copy link
Contributor Author

fullgame->appid was added from a previous PR d91a7c9
The "fake object fullgame" was not present in the response until now, since the method did not had the return statement.
In the case of portal2, appid will be null since it's already the "parent" game. But, if you take an another app, like a DLC, it can have a parent and his appid will be set.

isFree is actually returning false but print_r doesn't display it. I would recommend var_dump for your example since you get more information.

Only my opinon: I prefer empty data instead of something like $price->final = 'No price found'; because it's easier to manipulate afterward when we expect a double (or null) but get a string. The same goes with website, if it's null or empty string, we know it doesn't have one.

And thanks for your works, you saved me a lot of time 😄

@stygiansabyss stygiansabyss merged commit 594b763 into syntaxerrors:master Aug 21, 2018
@stygiansabyss
Copy link
Contributor

2.0.13 tagged with this change added in.

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