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

idTech2 shaders fix and compiler warning fixes #660

Merged
merged 5 commits into from Nov 8, 2020
Merged

Conversation

jdolan
Copy link
Collaborator

@jdolan jdolan commented Nov 3, 2020

This primarily addresses a bug where idTech2 games using the "shaders" API will receive Q3 / QER surface flags when a texture is applied to a face. Because these games are only using the "shaders" API for editor convenience (QER_TRANS is pretty nice), it's wrong to pass these flags down into the faces. QER_TRANS, for example, collides with SURF_LIGHT in Quake, Quake2, Quetoo, etc.

@@ -210,7 +210,7 @@ static picoModel_t *_fm_load( PM_PARAMS_LOAD ){
fm_xyz_st_t *triangle;
fm_frame_t *frame;

picoByte_t *bb, bb0;
picoByte_t *bb, *bb0;
picoModel_t *picoModel;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This seems like an obvious error. The incorrect storage type of this variable was causing 11 or so warnings that went away when I converted it to a pointer. This variable is passed to the Pico free function, so I'm fairly sure that's correct.

@@ -92,7 +92,7 @@ virtual void addType( const char* key, filetype_t type ){
virtual void getTypeList( const char* key, IFileTypeList* typelist ){
filetype_list_t& list_ref = m_typelists[key];

if (key == "model") {
if ( g_strcmp0( key, "model" ) == 0 ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another fairly obvious error my compiler picked up.

tex.flags = 0;
} else {
tex.flags = pCurrentShader->getFlags();
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And finally, here's the actual fix I set out to make.

@jdolan jdolan requested a review from TTimo November 3, 2020 04:01
@TTimo
Copy link
Owner

TTimo commented Nov 8, 2020

I don't have a setup to test this atm, but this looks straightforward - thanks! Hope you're doing well.

@TTimo TTimo merged commit f2d2da0 into TTimo:master Nov 8, 2020
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