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

SCI: Add City's fan Russian translation support #2673

Merged
merged 2 commits into from Jan 2, 2021
Merged

Conversation

@phcoder
Copy link
Contributor

@phcoder phcoder commented Dec 8, 2020

This PR has 2 commits. One to tolerate a slightly invalid image and another adds the hashes

@sev- sev- requested a review from bluegr Dec 8, 2020
@phcoder phcoder force-pushed the phcoder:torin branch from 6b4494e to 3158bcb Dec 8, 2020
@deepcode-ci-bot
Copy link

@deepcode-ci-bot deepcode-ci-bot bot commented Dec 8, 2020

Congratulations 🎉. DeepCode analyzed your code in 2.346 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@phcoder phcoder force-pushed the phcoder:torin branch from 3158bcb to e89643d Dec 8, 2020
@sluicebox
Copy link
Member

@sluicebox sluicebox commented Dec 15, 2020

Which specific resources have problems? Where can I find this fan translation?

I'd prefer that we use the resource patcher (resource_patcher.cpp) on the broken images instead of allowing the uncompressed version of getRow() to ignore invalid requests. That keeps game-specific workarounds out of the engine code, and I think that is even more important for fan stuff. That way we also have built-in documentation of which image/game/version is the problem. Right now this begs the question "why does the uncompressed reader allow this this but the compressed one doesn't?"

Separately, this is a good example of code that shouldn't be an assertion, since it's validating external data. I think it should be an error() if not a warning(), and should be the same in all the getRow() methods, but that can happen later.

@bluegr
Copy link
Member

@bluegr bluegr commented Dec 16, 2020

I agree with @sluicebox here. Since there are broken resources in this version, we should either try and patch them, or ignore specific errors for specific resources, not all of them.

Copy link
Member

@bluegr bluegr left a comment

Overall, the corrupt graphics should be identified and corrected, instead of silently dropping the errors. Perhaps it is possible to identify all the offending resources with external tools, e.g. SCI Companion, or SV

@phcoder phcoder force-pushed the phcoder:torin branch from e89643d to 8b4b428 Jan 2, 2021
@bluegr
Copy link
Member

@bluegr bluegr commented Jan 2, 2021

Excellent, thanks for addressing the issues! Merging

@bluegr
bluegr approved these changes Jan 2, 2021
@bluegr bluegr merged commit 3e7abf9 into scummvm:master Jan 2, 2021
3 checks passed
3 checks passed
Codacy Static Code Analysis Codacy Static Code Analysis
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deepcode-ci-bot Well done, no issues found!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.