Skip to content

Commit

Permalink
Brightmaps fix
Browse files Browse the repository at this point in the history
  • Loading branch information
Gaerzi authored and coelckers committed Jul 16, 2021
1 parent eb2646b commit 3f9a3a4
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/r_data/gldefs.cpp
Expand Up @@ -1155,7 +1155,7 @@ class GLDefsParser
if (lumpnum != -1)
{
if (iwad && fileSystem.GetFileContainer(lumpnum) <= fileSystem.GetMaxIwadNum()) useme = true;
if (thiswad && fileSystem.GetFileContainer(lumpnum) == workingLump) useme = true;
if (thiswad && fileSystem.GetFileContainer(lumpnum) == fileSystem.GetFileContainer(workingLump)) useme = true;
}
if (!useme) return;
}
Expand Down

5 comments on commit 3f9a3a4

@drfrag666
Copy link
Contributor

@drfrag666 drfrag666 commented on 3f9a3a4 Jul 17, 2021

Choose a reason for hiding this comment

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

The same check is used at line 1340 in void ParseMaterial() , that one should be changed too.

@alexey-lysiuk
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment lacks a permalink to source line in question, so everyone needs find it themselves just to confirm the case.
If you were spent some time on looking for similar errors, and to write the comment as well, how about making it useful?
Or even better, just make a PR with the fix.

@drfrag666
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/coelckers/gzdoom/blob/3f9a3a454b3889b1471dc851795098cd05ae4690/src/r_data/gldefs.cpp#L1340
Like this? You're not going to change it from GitHub anyway and you'll need to search for it to change it in your editor. The same line "if (thiswad && fileSystem.GetFileContainer(lumpnum) == workingLump) useme = true;". I also wrote it on the forum and Discord. I was about to include the commit in my last PR but i'm supposed to do one PR for only one thing. I mentioned function and line but it was wrong.

@alexey-lysiuk
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if I read your comment from a device that has full fledged web browser but no development tools?
Still, this conversation wouldn't happen you did it right initially. Pull requests and reviews exist for a reason.

Source code discussions without direct links to files and commits, patches instead of PRs, random thoughts instead of facts, inability to understand what others are asking, etc, look way too amateurish to me.
All this could be acceptable in some cases. Although, such things may become very annoying over time.
It's a generic observation, and I'm not taking about particular person. Everyone can make a mistake, but my complaint is about tendency to continue doing them over and over again.

@drfrag666
Copy link
Contributor

Choose a reason for hiding this comment

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

In the end i've made the PR:
#1434

Please sign in to comment.