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 Dreamscapes and Dreamscapes 2 #1211

Merged
merged 9 commits into from
Jun 20, 2020
Merged

Add Dreamscapes and Dreamscapes 2 #1211

merged 9 commits into from
Jun 20, 2020

Conversation

Starcommander
Copy link
Contributor

@Starcommander Starcommander commented Jun 10, 2020

Description

I added install scripts for 2 new Games

What works

Installing and playing on Linux and MacOs

What was not tested

I modified the scripts to match the new syntax from master branch.
That modification was not tested, i used Star_Craft2 as blueprint

Test

  • Operating system:
    Debian10 and 11 via flatpak
    Mac Os X 10.13.6 via dmg package
  • Hardware (GPU/CPU):
    On Debain11 I use a Radeon 5500 XT, that is not supported on flatpak --> software-rendering.

Ready for review

  • Script tested as a regular phoenicis user and working (if you have a problem -> create as draft and ask for help).
  • json-align and eslint run according to the documentation.
  • Codacy and travis checked.

@plata
Copy link
Collaborator

plata commented Jun 11, 2020

Please fix the Codacy issues and test the scripts before this can be merged.

@Starcommander
Copy link
Contributor Author

@plata Hello! I fear, there is nothing wrong with the json file.
In my opinion this is a "false positive"
Can you give me any hint, what this "Codacy issues" are complaining about?

The syntax is the same as on "Star Craft 2".
And what is that message "Format error: unexpected "" "??

Thanks in advance.

@plata
Copy link
Collaborator

plata commented Jun 11, 2020

This is indeed very strange. I've never seen such a behavior before. I've contacted Codacy to clarify.

@plata
Copy link
Collaborator

plata commented Jun 15, 2020

This should be: https://github.com/codacy/codacy-meta/issues/415

@plata plata mentioned this pull request Jun 16, 2020
@Zemogiter
Copy link
Contributor

Shouldn't it be one application per PR?

@plata
Copy link
Collaborator

plata commented Jun 16, 2020

Would be better, yes.

@Starcommander
Copy link
Contributor Author

Do I have to split it?
Or can you merge it at once?

@plata
Copy link
Collaborator

plata commented Jun 16, 2020

Normally one pull request per app. As the apps belong together, I would be ok with having an exception here. What's more important is that the scripts should be tested.

Apart from that: I've used this PR to test a work around for this strange Codacy behavior. Please merge upstream master to your fork such that the diff is shown correctly.

@Starcommander
Copy link
Contributor Author

I am not able to test any upstream-script because of PhoenicisOrg/phoenicis#2230
Is this a bug?

@Starcommander
Copy link
Contributor Author

@plata I was now able to test both scripts successfully.
Installs and runs with no issue.
But I had to change the freedesktop-version back as described in PhoenicisOrg/phoenicis#2230

@plata plata changed the title Add Dreamscapes and Dreamscapes2 Add Dreamscapes and Dreamscapes 2 Jun 20, 2020
Copy link
Collaborator

@plata plata left a comment

Choose a reason for hiding this comment

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

Shouldn't it be "Dreamscapes 2" (with space)?

"LINUX"
],
"testingOperatingSystems" : [],
"free" : false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The URL is free-games-download: Is it free or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is free of costs.
But it is not open source.
I will change it to true.
But I also suggest to distinguish between "free of cost" and free (regarding license).
So adding an additional field in future would be helpful, imho.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Feel free to open a new issue to discuss this. Currently, it's free as in beer (see docs).

"LINUX"
],
"testingOperatingSystems" : [],
"free" : false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above

@Starcommander
Copy link
Contributor Author

Personally I do not like spaces in file names and directories.
But it is your decision how it matches the rules of POL.
... in progress ...

@Starcommander
Copy link
Contributor Author

Adding space and change preferences to free: done.
I also tested the script with "Dreamscapes 2" and it works.

@plata plata merged commit b5c2c52 into PhoenicisOrg:master Jun 20, 2020
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.

3 participants