Lots of defect fixes #1262
Lots of defect fixes #1262
Conversation
| @@ -19,7 +19,7 @@ void vfsHDDManager::CreateEntry(vfsHDD_Entry& entry) | |||
| CreateBlock(entry); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Can we have an enum with error code and use it? Would make code easier to read.
There was a problem hiding this comment.
@vlj Sounds like it would make sense. I'll also make some return codes for the virtual memory helper functions.
|
im not sure, but it looks like this PR will break the arkedo series games (they will reach the menu, but wont go ingame) |
|
Why? |
|
@Ekaseo is there any new errors in the log? maybe before it was failing silently for some reason. |
|
Is there any progress on this ? |
|
@vlj It's rebased and ready for merge, from my point of view. Though, if there's any problems, I'd be happy to fix them. |
|
Is it ready for merging, @Nekotekina , or there is still something left? |
|
its been a while since i tested some builds, but unfortunately, nothing except a few samples work right now... do you ppl even test the stuff before merging it? (its not this PR that broke almost everything, it was something merged before...) for me, only cube samples work right now, everything else has an acess violation. If its only a problem with my pc, i will include some log files. |
|
@Ekaseo Same situation here. Almost everything dies with an access violation, except some of the most simple samples. |
|
What PR caused it? |
|
@Nekotekina #1274 seems to have broken almost everything. |
|
Which backend? |
|
@Ekaseo Also, don't forget to provide your settings. For example dx12 crashes if write color/depth buffer is enabled while oGL needs it to display many things correctly. |
|
@vlj But on #1272, with the almost EXACTLY same settings (excluding the frame limiter, oops :P): |
|
i use opengl and i use basic settings. i dont have anything special enabled, not even color/depth buffers. i mostly use the lowest resolution. i have vsynch one + framelimiter on auto. null on audio, have camera disabled, im using the default folder paths. I guess opengl has no support now compared to dx12. |
|
@Ekaseo Well, OpenGL does seem to be a bit of a mess. But I encounter the same errors with DX12. |
|
I'm intending to factor code between dx12 and GL very soon so that GL backend benefits from the work puts in dx12. |
|
tambry, I use default configuration in miscalleous parameters. |
|
@vlj I wasn't even aware of per game config now existing. Looks cool though. According to settings though, I do use the default configuration. (http://puu.sh/l4vDi/548f759efc.png) |
|
Still no merge ? |
|
It's conceptually bloating and introduces some errors. Most of those functions results are assumed to be "OK" and typing so much unique code to "report" an error and increasing complexity returning various error codes... I don't like it. I would use CHECK_ASSERTION macro for them (it's unrelated to |
| @@ -15,27 +16,30 @@ namespace memory_helper | |||
| void* reserve_memory(size_t size) | |||
| { | |||
| #ifdef _WIN32 | |||
| return VirtualAlloc(NULL, size, MEM_RESERVE, PAGE_NOACCESS); | |||
| void* ret = VirtualAlloc(NULL, size, MEM_RESERVE, PAGE_NOACCESS); | |||
| CHECK_ASSERTION(ret == NULL); | |||
There was a problem hiding this comment.
CHECK_ASSERTION accepts "right" condition, in your code it's inverted everywhere.
Plus many return conditions are actually wrong.
|
@Nekotekina Should be fixed now. If there are any other issues, please let me know. |
|
You compare unsigned values which cannot be < 0 |
|
@Nekotekina Seems like most of those comparisons were introduced by function return type being unsigned, while the return values were negative. I have fixed those return types in a few places and also added the CHECK_ASSERTION in a few more places. Are there any issues left? |
|
Who asked you to change return type? Much better and always works. |
| memcpy(npdrm_key, klicensee_key, 0x10); | ||
| // FIXME: Check is always false. | ||
| /*if (klicensee_key != NULL) | ||
| memcpy(npdrm_key, klicensee_key, 0x10);*/ |
There was a problem hiding this comment.
@Nekotekina What do you mean? This check is always false, thus I commented it and added a "FIXME" note. If you want me to uncomment me it, I can do that.
There was a problem hiding this comment.
It better be kept commented (though it could use // comments) with remark. If we keep it the old way everyone will forget about what this check intended to do.
Maybe more elaborate comment would be better.
There was a problem hiding this comment.
@danilaml Should be better now.
| { | ||
| return -1; | ||
| } | ||
|
|
There was a problem hiding this comment.
This one is fixed correctly on my branch.
Also fix some Seek methods return types being unsigned, while returning negative errors. Added the CHECK_ASSERTION macro checks in a couple more places. Simplified CHECK_ASSERTION macro usage.
It can be enabled in options, under C/C++ -> Editor. It's experimental, but should be faster. It will be introduced in Update 1.
|
@Nekotekina Are there any other issues blocking this from being merged? |
Includes a few memory corruption bugs fixed, added initialization for a couple variables, fixed some flow control problems and added error checks and error logging to a lot of places.
HDD functions now also return a negative version, if they fail seeking. Didn't add any checks for when they're actually used, but if we'd want to be more or less correct, we should check for errors from those functions in the future.