Skip to content
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

Rcheevos #84

Merged
merged 5 commits into from
Mar 26, 2022
Merged

Rcheevos #84

merged 5 commits into from
Mar 26, 2022

Conversation

NikosSiak
Copy link
Member

This PR adds support for rcheevos to RetroPlayer. This PR is used by xbmc/xbmc#20913 and depends on it to be build

src/cheevos/Cheevos.cpp Outdated Show resolved Hide resolved
src/cheevos/Cheevos.cpp Outdated Show resolved Hide resolved
char _hash[HASH_SIZE] = {};

int res = rc_hash_generate_from_file(_hash, consoleID, filePath.c_str());
hash = _hash;
Copy link
Member

@garbear garbear Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hash should only by set if res == RC_OK. However, on the note of checking return value, here's a snippet of (maybe outdated) documentation I found:


Return value

If the function succeeds, the return value is RC_OK. Otherwise, the error code can be converted to a string using rc_error_str.

  • RC_INVALID_STATE - one or more required parameters was not provided.
  • RC_OUT_OF_MEMORY - enough memory could not be allocated to complete the operation.

To be fully correct, we should handle all pre-conditions and post-conditions when calling into an external library. To satisfy post-conditions, this means checking the result and logging errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments above the function state:

/* generates a hash from a file.
   * returns non-zero on success, or zero on failure.
   */

That's why I didn't use the RC_OK you suggested in the previous comment, it was a bit confusing to check if the result is different from the OK value my thoughts when reading that were that if CCheevos::GenerateHashFromFile returns true it means it failed

src/libretro/libretro.h Outdated Show resolved Hide resolved
src/libretro/MemoryMap.h Outdated Show resolved Hide resolved
src/libretro/MemoryMap.h Outdated Show resolved Hide resolved
@garbear
Copy link
Member

garbear commented Feb 7, 2022

I scrutinized the code and found a couple suggestions.

@NikosSiak NikosSiak force-pushed the rcheevos branch 5 times, most recently from fcffb19 to 0a3df37 Compare February 13, 2022 16:37
@garbear
Copy link
Member

garbear commented Feb 15, 2022

I tried compiling on Windows and found the following warnings:

[ 11%] Building CXX object CMakeFiles/game.libretro.dir/src/cheevos/Cheevos.cpp.obj
Cheevos.cpp(242): warning C4267: '-=': conversion from 'size_t' to 'unsigned int', possible loss of data
Cheevos.cpp(245): warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss of data
Cheevos.cpp(248): warning C4267: '-=': conversion from 'size_t' to 'unsigned int', possible loss of data
Cheevos.cpp(250): warning C4267: '+=': conversion from 'size_t' to 'unsigned int', possible loss of data

[ 25%] Building CXX object CMakeFiles/game.libretro.dir/src/libretro/MemoryMap.cpp.obj
MemoryMap.cpp(14): warning C4018: '<': signed/unsigned mismatch

@garbear
Copy link
Member

garbear commented Mar 4, 2022

Just pointing out, the last two commits should be squashed so that the build succeeds on every commit to aid in bisecting. But no need to update for this PR.

NikosSiak and others added 5 commits March 24, 2022 11:44
Rcheevos expose hash and url functions

RCheevos expose post rc_url_ping function

fix crash when cheevos is deleted

Added comment to the base file of RetroArch

Reset rc_runtime

Fix compiler error and refactor Peek()

Guard against uninitialized variable
Replace libretro.h with libretro-common/libretro.h
@NikosSiak NikosSiak closed this Mar 26, 2022
@NikosSiak NikosSiak reopened this Mar 26, 2022
@NikosSiak NikosSiak merged commit 8952f3c into kodi-game:Nexus Mar 26, 2022
@garbear garbear mentioned this pull request Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants