-
-
Notifications
You must be signed in to change notification settings - Fork 22
first attemps at CLI file support #14
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
Conversation
Got it working. Still unsure if I am doing it correctly, as I am mostly guessing and copying from Swiss. But it is tested and working on my DOL-101. BTW, I made this little DOL to test with. All it does is print the contents of |
Before I start reviewing this, I just want to let you know that while I'm going to accept this feature, I would like you to open an issue so we can discuss things before you spend your time implementing more features. Otherwise it's not really a minimal IPL implementation anymore, is it? 😛 CLI support in particular is only really useful for GBI. I know some people only use their GameCube to capture GameBoy footage, so it's a valid use case and this doesn't add too much code, but we're already starting to copy-paste code from Swiss which is a good sign that we should start drawing the line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks alright. The mutable global state is really starting to grow out of control though. It was fine when iplboot was simpler, but if we're going to add any more features then the whole thing is due for a refactor.
source/main.c
Outdated
// Wait to exit while the d-pad down direction is held. | ||
while (all_buttons_held & PAD_BUTTON_DOWN) | ||
{ | ||
VIDEO_WaitVSync(); | ||
PAD_ScanPads(); | ||
all_buttons_held = ( | ||
PAD_ButtonsHeld(PAD_CHAN0) | | ||
PAD_ButtonsHeld(PAD_CHAN1) | | ||
PAD_ButtonsHeld(PAD_CHAN2) | | ||
PAD_ButtonsHeld(PAD_CHAN3) | ||
); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the whole if (dol_argc)
out of if (dol)
and you won't need to extract this bit into a function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea this whole block feels awkward. Although we know that currently there is no way dol_argc
can be set without dol
, it felt disconnected to have logic that reads as if dol_argc
could be processed even if dol
is not set. Feels weird to atomize the loading of the args and the loading of the DOL as the former doesn't make sense without the later.
I did it this way because I assumed there two distinct exit paths (with and without a DOL) which could not converge because of SYS_ResetSystem
/SYS_SwitchFiber
. I don't know exactly how these interplay.
Given that SYS_ResetSystem
is not directly related to the loading of the DOL, we could move it the end. This is only possible if it's OK to have to call SYS_ResetSystem
after SYS_SwitchFiber
and after a delay. Something like:
bool should_reset = false;
if (!dol) goto exit;
if (dol_argc)
{
// ... load args
}
// ... load dol
SYS_SwitchFiber(...);
should_reset = true;
exit:
// wait for d-pad down to be released
if (should_reset)
{
SYS_ResetSystem(SYS_SHUTDOWN, 0, FALSE);
}
return 0;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure this particular call is to shut down the libogc "OS" before jumping into the relocation stub. @Extrems will have to clarify this.
Returning from main otherwise returns to libogc's startup code which will do its own cleanup and then a hard reset (which is how we reboot into the original IPL, in fact it's the only way because you can't reset and reactivate the bootrom decryption block from software). Technically we could read, decrypt and execute the original IPL like a regular DOL, but that's really unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only possible if it's OK to have to call SYS_ResetSystem after SYS_SwitchFiber and after a delay.
That absolutely isn't ok. The operating system must be shutdown before jumping into the relocation stub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured something like that was the case.
Working back from there, we do indeed have 2 separate and non-converging exit paths. We can't wait for d-down before the split as one of the paths does some logging. And we can't wait after because one of the paths never returns.
The solution is to either mix the paths or allow each path to determine when to wait. I went with the later. If I were to refactor I would do something like:
if (!dol)
{
delay_exit();
return 0;
}
if (dol_argc)
{
// ... load args
}
// ... load dol
delay_exit();
SYS_ResetSystem(...);
SYS_SwitchFiber(...);
return 0;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed the refactor described above.
LT;DR: You won't hurt my feelings denying a pull request.
I've been building features to solve problems that I have and then opening pull requests if I think the project at whole could benefit. Solves my issues and I have fun doing so. As such, I would not be offended if you were to reject a pull request. I wouldn't conciser it wasted time or effort. It's more "I built this thing. Do you want it too?" and less "I built this for you. Please take it." (you being iplboot). That said, I am happy to start my ideas with an issue so discussion can happen before/while I am implementing; allowing for the possibility to discover how the problem I am trying to solve may align with a more global iplboot problem set and solution. Yea we will hit that line at some point. I think the minimal aspect is still important for simplicity's sake. As you have mentioned before, the vast majority of people are likely going to have the simplest use-case: Just start Swiss. So it becomes a game of not adding to much frosting and sprinkles to that cake. Example: If you hold a button during boot, FYIY, this was the last missing feature for me. My list is complete:
That said, I do have some ideas that do not fit in iplboot. For example:
|
Glad we're on the same page. It's less about hurting your feelings and more about me not wasting your time as a contributor, as well as setting boundaries for what is and isn't acceptable for the project. I've had contributors react poorly to their PRs being rejected before and I just don't want anyone to feel discouraged from contributing just because their first PR was deemed out of scope.
This is something I would like to see for casual use, but the complexity of implementing this while also working around Nintendo's copyright and supporting all versions of the IPL is definitely out of scope for this specific project. On top of that, because iplboot was in part intended for homebrew development, the quick boot time and integrated USB boot server can save a lot of time in an edit compile run cycle. It is possible to reconcile the two, but I'm leaving that for another project down the line. |
Yup, super lofty. Not sure it's even feasible. More of personal/research project. Those last 3 were explicitly non-iplboot ideas. |
5a27ce7
to
1373e6f
Compare
Rebased upstream. Included |
Unrelated, but I can't find the reverse engineered BS2 now. I remember seeming some things about it on Twitter. Specifically the GameCube animation in different colors. You guys know anything? EDIT: Found it: https://twitter.com/trevorrudolph/status/1539659171653890050 |
I think we're good to go. |
Ported CLI file support from Swiss. Although, I have no idea how to actually pass the arguments to the loaded DOL.
Can I get some direction?