benzammour / cretro

👾 cretro - Yet another Chip 8 Emulator!
MIT License
4 stars 2 forks source link

feat: enhance logging and parse cli arguments #10

Closed mormod closed 1 year ago

mormod commented 1 year ago

In this pull request, a rudimentary version of a debug window will be implemented. This resolves (part of) #3.

This pull request adds a nicer way to log to the console as well as the ability to take command line arguments via flags and optional parameters. Using this, flags for setting the debug level

-d <0..4> (default: 4)

as well as setting the frequency of the emulated CPU

-f <1-1000000> (default: 1000000)

which previously was a mandatory argument, were introduced. New usage is thus:

cretro [-d <0..4>] [-f <1-1000000>] <ROM>

Logging levels are:

mormod commented 1 year ago

As of now, I implemented a nicer way to log debug messages. This is part of the debugging experience, but has not really much to do with a debug window. I would suggest to merge this (anyway should you want to) without having implemented the actual window, such that that task can be done cleanly in a new PR. @Benzammour

Title and description can be changed accordingly.

marcluque commented 1 year ago

I think that debug.c/.h should be renamed to something like log oder logging since debug might suggest that it deals with the debug window. A distinction like debug_logging and debug_window could be possible too.

marcluque commented 1 year ago

I think that maybe the getopt switch case could parse all arguments. That would make the parsing more consistent. The check whether required parameters are present could then be done afterwards. This could ofc be paired with the suggestion I made above to check whether argc >= 2.

mormod commented 1 year ago

I think that maybe the getopt switch case could parse all arguments. That would make the parsing more consistent. The check whether required parameters are present could then be done afterwards. This could ofc be paired with the suggestion I made above to check whether argc >= 2.

As far as I'm aware, getopt shall only be used to parse optional arguments. Since frequency and __rom_path__ are not optional, these should not be parsed directly by getopt. In addition, getopt is configured using a parsing string in the format "ab:c::", which would mean

If frequency and path to ROM shall be parsed by getopts, we would have to introduce flags for those arguments, making them optional. This, however, would only make sense for the frequency, but not for the path to ROM, as this emulator is quite useless without a ROM loaded.

marcluque commented 1 year ago

I see 🤔 Making frequency an option sounds like a good idea while keeping the ROM path, as you suggested, an argument