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

STARK: Fix bug #14482 Texture mods with unusual names not loading #5249

Closed
wants to merge 0 commits into from

Conversation

gordonfreeman01
Copy link
Contributor

STARK: Fix bug #14482 Texture mods with unusual names not loading

Addresses the following bug by re-encoding the special character causing it:
https://bugs.scummvm.org/ticket/14482

@@ -117,7 +117,7 @@ void TextureSet::extractArchive() {
}

Gfx::TextureSet *TextureSet::readOverrideDdsArchive() {
Common::String archiveName = _filename + ".zip";
Common::String archiveName = _filename + ".zip";
Copy link
Member

Choose a reason for hiding this comment

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

Why? Please revert this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? Please revert this

Huh, this is weird, I didn't actually modify that myself (doesn't show up as a diff in VS either). Any idea what might be causing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got the problem, whitespace compares were off in VS, fixed in new PR #5251 as Github closed this one after I rebased my branch.

// \x91 is ae in CP437 and CP850 which is used by some compression utilities
// \xc3\xa6 is the UTF-8 2-byte representation
// Both get converted to the game's original encoding here
if (textureName == "pupp\x91""r" || textureName == "pupp\xc3\xa6""r") {
Copy link
Member

Choose a reason for hiding this comment

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

Why are you doubling the quote here?
It doesn't seem to be needed as per below.
Either you separate the "r" in every places either you don't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why are you doubling the quote here? It doesn't seem to be needed as per below. Either you separate the "r" in every places either you don't.

I implemented feedback from @sev- , personally I think it would be more readable to have it in one block but will go with the project consensus. Would a single set of quotes be preferred?

Copy link
Member

Choose a reason for hiding this comment

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

single quote is preferred

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok cool, that's done in the latest version of the branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in new PR #5251 as Github closed this one after I rebased my branch.

@gordonfreeman01
Copy link
Contributor Author

@bluegr @lephilousophe I attempted some updates based on your feedback and rebased my branch but it ended up closing the PR automatically as a result, apologies. Is there any other process used to update the branch from master and making a PR? I was pulling from master then rebasing my changes on that updated mirror of master for this PR but any rebase seems to end the PR and merge commits are against wiki instructions.

Concerning the issue raised by @bluegr I didn't edit the tabs in the and the encoding seems to match with other source files, any idea what might be causing the tab to be removed during push?

@gordonfreeman01
Copy link
Contributor Author

@bluegr @lephilousophe I attempted some updates based on your feedback and rebased my branch but it ended up closing the PR automatically as a result, apologies. Is there any other process used to update the branch from master and making a PR? I was pulling from master then rebasing my changes on that updated mirror of master for this PR but any rebase seems to end the PR and merge commits are against wiki instructions.

Concerning the issue raised by @bluegr I didn't edit the tabs in the and the encoding seems to match with other source files, any idea what might be causing the tab to be removed during push?

Please ignore, the tab issue was just a VS setting to ignore whitespace diffs mistakenly left on. For anyone else reading this, I created a new #5251 as Github closed this one after I rebased my branch, individual threads have been updated with the link to the new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants