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

Cdrom misc #1023

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Cdrom misc #1023

wants to merge 7 commits into from

Conversation

johnbaumann
Copy link
Collaborator

Fixing up some cdrom behavior around command responses. The main focus is currently around disc type detection and to avoid sending the license string for a non-bootable disc, though ideally this would not prevent reading from "unlicensed" discs, i.e. an asset CD when paired with a manually launched binary.

@codecov
Copy link

codecov bot commented Nov 7, 2022

Codecov Report

Base: 16.38% // Head: 16.39% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (02db7ef) compared to base (8955f1d).
Patch coverage: 5.88% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1023      +/-   ##
==========================================
+ Coverage   16.38%   16.39%   +0.01%     
==========================================
  Files         362      362              
  Lines      115768   115785      +17     
==========================================
+ Hits        18971    18986      +15     
- Misses      96797    96799       +2     
Impacted Files Coverage Δ
src/cdrom/cdriso.cc 7.76% <0.00%> (-1.33%) ⬇️
src/cdrom/cdriso.h 33.33% <0.00%> (-3.04%) ⬇️
src/core/cdrom.h 37.50% <0.00%> (-2.50%) ⬇️
src/core/cdrom.cc 7.28% <6.25%> (+0.20%) ⬆️
src/cdrom/iec-60908b.h 3.63% <0.00%> (-14.55%) ⬇️
src/core/r3000a.cc 59.91% <0.00%> (+6.16%) ⬆️
src/core/r3000a.h 62.31% <0.00%> (+7.24%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

m_result[1] = 0;
m_result[2] = 0;
m_result[3] = 0;
memset((char *)&m_result[1], 0, 7);
Copy link
Member

Choose a reason for hiding this comment

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

Might be easier / safer to do the following:

memset(m_result, 0, sizeof(m_result);
m_result[0] = m_statP;

break;
}
} else {
IO<File> psxexe(reader.open("PSX.EXE;1"));
if (!psxexe->failed()) {
m_cdromId = "SLUS99999";
exename = "PSX.EXE;1";
m_bootable = true;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe the check function should display a warning at the end that the disc isn't bootable in case this boolean is still false.

@nicolasnoble
Copy link
Member

FWIW, asset discs exist on the PS1. They'll usually contain a dummy binary that displays a warning about the disc being a "DLC".

For example, beatmania APPEND 3rdMIX.

@johnbaumann
Copy link
Collaborator Author

Superseded by #1129

Leaving branch/PR in-tact until the new code is mostly working.

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