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

total rewrite of Doom3 script #1869

Merged
merged 10 commits into from
May 25, 2022
Merged

total rewrite of Doom3 script #1869

merged 10 commits into from
May 25, 2022

Conversation

theofficialgman
Copy link
Collaborator

@theofficialgman theofficialgman commented May 24, 2022

upstream @techcoder20 was not fixing their app and the repo there is separate from pi-apps

move the install to pi-apps, fix multiple broken things, and enable 64bit (since it should have always worked). this is necessary as Doom3's broken installed sdl2 packages have been blocking multiple other pi-apps apps from being merged

edit: also, yes, apparently I can't spell lately. the branch is misspelled

upstream @techcoder20 was not fixing their app and the repo there is separate from pi-apps

move the install to pi-apps, fix multiple broken things, and enable 64bit (since it should have always worked)
doesn't actually exist in debian buster/bullseye (might be in raspbian from stretch)
@theofficialgman
Copy link
Collaborator Author

a lot of these packages appear to be unnecessary, and were likely to due to compiling SDL2 which is not done anymore (its in the deb repos)

apps/Doom 3/install Outdated Show resolved Hide resolved
@Botspot

This comment was marked as outdated.

@Botspot
Copy link
Owner

Botspot commented May 24, 2022

I think this looks good. Not sure why this PR is a draft, but you can merge when ready.

@theofficialgman
Copy link
Collaborator Author

its a draft because I want somebody running piOS buster 32bit (as well as 64bit on buster or bullseye) to test out running doom3 before merging

you can use the demo, thats fine for testing.

@theofficialgman
Copy link
Collaborator Author

theofficialgman commented May 25, 2022

@Botspot keep in mind I've also marked multiple of the new (and old) PRs as draft due to Doom 3 improper sdl2 packages causing conflicts. this PR needs to be tested, merged, and a runonce added (to remove the packages even if the user doesn't upgrade doom3) to fix these issues

the list of PRs in waiting for this merge is:
#1359
#1874
#1875
#1868

basically Doom3's bad packages have been blocking all SDL2 apps from pi-apps and a users install

@Botspot
Copy link
Owner

Botspot commented May 25, 2022

this PR needs to be tested, merged, and a runonce added

Wow. I was not aware the current script caused so many problems! Doom 3 has 13k users, so I'm honestly surprised we haven't received more complaints about this.

@Botspot
Copy link
Owner

Botspot commented May 25, 2022

and a runonce added (to remove the packages even if the user doesn't

Does this look good?

#Doom3 app by techcoder20 installed problematic SDL2 packages. (sdl2-image sdl2-mixer sdl2-ttf)
#Nothing else uses them, and they just cause problems with many other games, so remove them
runonce <<"EOF"
  if dpkg -l sdl2-image sdl2-mixer sdl2-ttf &>/dev/null ;then
    sudo_popup apt purge -y sdl2-image sdl2-mixer sdl2-ttf
  fi
EOF

@theofficialgman
Copy link
Collaborator Author

theofficialgman commented May 25, 2022

  if dpkg -l sdl2-image sdl2-mixer sdl2-ttf &>/dev/null ;then
    sudo_popup apt purge -y sdl2-image sdl2-mixer sdl2-ttf
  fi

this only works if all of them are installed. for the edge case where only one or two of them are installed, thats why I did what I did in the script (I know its ugly but I couldn't find a better way)

https://github.com/Botspot/pi-apps/pull/1869/files#diff-2654fa8ffaa944920866f93e087b47b91ecfdab13e23f965fa27722bd8bd7f89R14-R26

@Botspot
Copy link
Owner

Botspot commented May 25, 2022

  if dpkg -l sdl2-image sdl2-mixer sdl2-ttf &>/dev/null ;then
    sudo_popup apt purge -y sdl2-image sdl2-mixer sdl2-ttf
  fi

this only works if all of them are installed. for the edge case where only one or two of them are installed

Good point. If any of the 3 packages are missing then dpkg will return a non-zero exit code. I will do this then:

runonce <<"EOF"
  if dpkg -l sdl2-image &>/dev/null || dpkg -l sdl2-mixer &>/dev/null || dpkg -l sdl2-ttf &>/dev/null ;then
    sudo_popup apt purge -y sdl2-image sdl2-mixer sdl2-ttf
  fi
EOF

@theofficialgman
Copy link
Collaborator Author

theofficialgman commented May 25, 2022

Good point. If any of the 3 packages are missing then dpkg will return a non-zero exit code. I will do this then:

runonce <<"EOF"
  if dpkg -l sdl2-image &>/dev/null || dpkg -l sdl2-mixer &>/dev/null || dpkg -l sdl2-ttf &>/dev/null ;then
    sudo_popup apt purge -y sdl2-image sdl2-mixer sdl2-ttf
  fi
EOF

@Botspot that still will not work. purge requires that all of those packages be installed in order to purge. again, thats why I did what I did

you can see this yourself without needing to install those bad pacakges, just test with something you normally have installed

sudo apt purge -y sdl2-image sdl2-mixer sdl2-ttf geany
Reading package lists... Done
Building dependency tree       
Reading state information... Done
E: Unable to locate package sdl2-image
E: Unable to locate package sdl2-mixer
E: Unable to locate package sdl2-ttf

(it ends here without purging)

@Botspot
Copy link
Owner

Botspot commented May 25, 2022

I've added the runonce in commit b0dbf99

Can this PR be merged now?

@Botspot
Copy link
Owner

Botspot commented May 25, 2022

its a draft because I want somebody running piOS buster 32bit (as well as 64bit on buster or bullseye) to test out running doom3 before merging

I forgot to mention earlier: I tested this PR and it worked fine, so merging this now.

@Botspot Botspot marked this pull request as ready for review May 25, 2022 20:52
@Botspot Botspot merged commit 0c1ea6b into master May 25, 2022
@Botspot Botspot deleted the doom3-rewordk branch May 25, 2022 20:52
@theofficialgman
Copy link
Collaborator Author

I forgot to mention earlier: I tested this PR and it worked fine, so merging this now.

ah... well thats what I was waiting on. with the amount of users doom3 has, we shouldn't have to wait long if this doesn't work for some reason

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