PS5NorModifier icon indicating copy to clipboard operation
PS5NorModifier copied to clipboard

Codebase cleanup

Open ScrubN opened this issue 7 months ago • 7 comments

I stumbled across this project from the Louis Rossmann video and decided to check it out for the hell of it. I noticed some of the code would be hard to modify as-is and would benefit from a bit of cleaning, but wanted to ask for permission before making a pull request.

My current plan is to put both projects under the same solution and simplify some complex or highly nested code, such as ParseErrors in UART-CL: https://github.com/TheCod3rYouTube/PS5NorModifier/blob/3064e07b65c3387d378995a653742484adf2d50b/UART-CL%20By%20TheCod3r/UART-CL%20By%20TheCod3r/Program.cs#L90-L153

Because both projects accomplish the same goals, some code could also be extracted to a common core library.

Please let me know what you think :)

ScrubN avatar May 09 '25 01:05 ScrubN

+1 I think a Library which handles the data parsing part for the consoles (ps3-ps5) would be great, makes it easier to maintain and implement in other projects. Since I work on a cross platform tool which abstracts the hardware specific stuff (all of the different programmers around) for handling the flash that would allow me to implement the functions for handling the data - making everything from dumping/flashing and handling the data a single application :)

DSchndr avatar May 09 '25 07:05 DSchndr

Seems that the majority of the code that actually does something is here and here

Ignore the other stuff, it's trivial and not important. I'd suggest taking that code, and making it into a separate class.

The logic, that is. Not the file opening and browsing and verious GUI-stuff

So for example, after about 5 minutes of guessing

public enum Ps5Edition : byte
{
    Disk = 0x03,
    Digital = 0x02
}

[StructLayout(LayoutKind.Sequential, Pack = 1)]
public readonly struct Ps5VersionInfo
{
    public readonly byte Version; // year?
    public readonly Ps5Edition Edition;
    public readonly short Field3; // unknown
    public readonly int Field4; // unkown
    public readonly int Field5; // unkown

    public const int Offset = 0x1c7010;

    
}


public class PsNorModifier : IDisposable
{
    public PsNorModifier(Stream inputStream)
    {
        _inputStream = inputStream;
        _reader = new BinaryReader(_inputStream, Encoding.UTF8, true);
    }

    private readonly Stream _inputStream;
    private readonly BinaryReader _reader;


    public async Task<Ps5VersionInfo> ReadVersionInfo()
    {
        _inputStream.Seek(Ps5VersionInfo.Offset, SeekOrigin.Begin);
        var sz = Marshal.SizeOf<Ps5VersionInfo>();
        var buffer = new byte[sz];
        var read = await _inputStream.ReadAsync(buffer);
        if(read != sz)
            throw new FormatException("Unexpected number of bytes read");
        return Unsafe.ReadUnaligned<Ps5VersionInfo>(ref buffer[0]);
    }

    public void Dispose()
    {
        _reader.Dispose();
        _inputStream.Dispose();        
    }
}

As an example on how to read the version information. Fyi I know nothing about PS5 ROM dumps, and I have not tested this code, it's just off the top of my head.

I'd use it like so :


using var infoReader = new PsNorModifier(new FileStream("example.bin", FileMode.Open, FileAccess.Read));

var version = await infoReader.ReadVersionInfo();

if(version.Edition == Ps5Edition.Digital)
    Console.WriteLine("Congrats, it's a digital edition :( ");

Knowing more about the file format itself would of course be a lot of help

Edit : I'm glad we have people like Louis Rossmann and @TheCod3rYouTube who actually tries to do something about the unconscionable and corrupt nature of things

intbeam avatar May 09 '25 07:05 intbeam

I think @TheCod3rYouTube should consider his intentions with this project and set expectations. He made something really cool and gave it to the world. That alone is amazing.

The key question is whether he continue maintenance and whether he will accept PRs. It's perfectly fine not to do that, the software work, but it would be better to set expectations in readme/issue template/sticky issue.

If he doesn't, there is an option of adding a member with commit rights who can do that will help with maintenance that instead. Generally, the best candidates are people who make quality PRs.

Right now, he got the influx of people and it is an opportunity. The reality is that many projects end up with no commits in years and 20 PRs and 50 issues.

jahav avatar May 09 '25 16:05 jahav

Hi all, swinging through to see how I can contribute - glad to see it's in C#, which I've been doing for a ... while 😂

Anyway, +1 to @jahav 's comment above; we need to either have a decision from @TheCod3rYouTube on contributions (format, checks, quality gates, whatever) so we're not just merging things in willy-nilly, or else have a discussion with him and anyone else who wants to be involved, and see if we can have consensus. Whether that's a wiki thread, a discussion issue, or whatever; it could be fun!

What are we doing, where are we going, and what do we want it to look like, basically 😄

thanasip avatar May 12 '25 12:05 thanasip

Hi everyone,

same for me. Louis Rossman's latest video motivated me to contribute here. After looking through PR #18 I'll be holding back until there is a decision on merging that. I'd otherwise probably be duplicating much of the work in there for reasons mentioned above. Leaving this here to voice my support, I'll be happy to jump in once things move forward.

florianknecht avatar May 12 '25 14:05 florianknecht

FYI I've submitted #33 which is a full rewrite, more or less

RJohnsonPGH avatar May 15 '25 01:05 RJohnsonPGH

FYI I've submitted #33 which is a full rewrite, more or less

I see that, and so have a few others.

Because of this, TheCod3r is now forced to merge the several smaller fixes then wait for someone with a big refactor PR to merge those fixes into their branch, or only merge a single big refactor PR and reject everyone else. This is exactly the situation I wanted to avoid by making this issue.

ScrubN avatar May 15 '25 03:05 ScrubN