ntrteam / flashcart_core

A hopefully reusable component for dealing with flashcart specific behavior.
GNU General Public License v3.0
128 stars 32 forks source link

Cart improvements #64

Closed kitlith closed 6 years ago

kitlith commented 6 years ago

The majority of improvements from #61 without the big templates / loop deduplication. I think.

We should be able to cherry-pick commits from here into the master branch if they don't all work out, as well.

The last commit adds a logging API, not from #61, which I've been wanting for a while now. (and have been using for testing here and there)

handsomematt commented 6 years ago

Nice, everything here seems really good. Good usage of weak attributes to not enforce an implementation. The logging API seems fine too, and general refactoring all seems fine.

I can't see any obvious errors here at all, only massive improvements to each implementation.

kitlith commented 6 years ago

right, I forgot: want to add a priority parameter to the logging API. one sec.

handsomematt commented 6 years ago

:thumbs_up:

Fachher commented 6 years ago

Wouldn't it be better to replace

define PAGE_ROUND_UP(x, s) ( ((x) + (s)-1) & (~((s)-1)) )

define PAGE_ROUND_DOWN(x, s) ( (x) & (~((s)-1)) )

define BIT(n) (1 << (n))

by inline functions??? This code looks more like C then C++

handsomematt commented 6 years ago

None of us are C++ experts other then to use the classes to make this project more extensible.

You can always PR though :)

kitlith commented 6 years ago

Hah. Yeah, pretty much. We use malloc and printf... etc. Using it a lot like C with classes. Feel free to PR!

I will note that C also has inline functions, yet common practice is still to define those as macros. shrug