Broken shaders on Atlantica map #271
Comments
|
So long as you fix it by still keeping the optimized code, I don't care. |
|
I've narrowed down the problem to Shader_CompressBracedSection in OpenJK. It allows shaders to have names with newlines in them and it parses the shader above as "#################### The best way to fix this would probably be to skip a name when a newline is found, give a warning, then try to parse a name again (until it finds a starting brace for the shader). |
|
Given that you guys want to keep "backward" compatibility, I think the shader parsing should remain able to deal with such lines. Also,just noticed that shader syntax errors that have too many }} cause OpenJK to crash on startup. I think in baseJA it just messed up the shaders ingame. Perhaps itd be nice to add a warning to the crash log so people would know what had caused it? |
|
Whether it should "deal with them" is irrelevant, it isn't simply dealing with them or treating them as comments, just seems to know that it isn't a shader because it gobbles it all into 1 giant buffer which somehow still works. That crash shouldn't happen either and is probably tied to also having ###s in the file which is causing the parser to break as it is. And there isn't a crashlog because you are running a "Final" build. aka no debug symbols. |
|
Huh? If you want to maintain backward compatibility, OpenJK should be able to read and use (properly) any shader files that baseJA can. Or have you ditched that goal? As for the crash, that has nothing to do with # or Atlantica's shader, just any extra { or } in any shader file. And it appears to be fixed in the latest revision I downloaded so you can ignore that part now - sorry. |
|
You shouldn't be using ### period, so... Sent from my Windows Phone From: SzicoVIImailto:notifications@github.com Huh? If you want to maintain backward compatibility, OpenJK should be able to read and use (properly) any shader files that baseJA can. Or have you ditched that goal? As for the crash, that has nothing to do with # or Atlantica's shader, just any extra { or } in any shader file. And it appears to be fixed in the latest revision I downloaded so you can ignore that part now - sorry. Reply to this email directly or view it on GitHub: |
|
The point is that even if JKA wasn't meant to accept comments like that, it's what's expected from a user point of view based on past experience. And anyway, what's wrong with supporting #s in shaders? We don't fix poking/wiggling in the saber system, even though it's considered an exploit because it's what's expected. Same principal here. And anyway, it shouldn't be crashing either way :p |
|
^^What he said. |
|
I'm not really saying there's anything wrong with that. But I think it's ridiculous that we need to accept input from files which use black magic/bugs in order to work correctly. The system was never designed to allow for ## comments. Sent from my Windows Phone From: Alex Lomailto:notifications@github.com The point is that even if JKA wasn't meant to except comments like that, it's what's expected from a user point of view based on past experience. And anyway, what's wrong with supporting #s in shaders? Reply to this email directly or view it on GitHub: |
ghost
assigned
|
Well if it doesn't read them then it ceases to be backward-compatible really doesn't it? I'm sure there are other mods that use that kind of syntax since I'm pretty sure that's where I decided to lay it out like that came from. |
Technically, yes. Personally I'd be fine with adding |
|
@SzicoVII just out of curiosity where did you come up with the idea that ###s were comments? It's been pretty well known that C/C++ style // and /* ... */ have been the valid comment style in q3 engine for years shaders not excluded. We aren't actively trying to give reasons its not going to be fixed to "work". Just trying to look for a better fix other than allowing just ###s as comments. @xycaleth thats a pretty piss poor comparison tbh. And I HAD fixed it that way by allowing them to be comments until Didz wanted to investigate further. |
|
@ensiform and then continue parsing so that any modders will be notified while testing or creating their maps and shaders and will fix them. It's a bit of a win-win. Players can still play, but they'll get an annoying error, and map developers will either get pestered by players or by the error and fix it. |
|
Yea go ahead and write that into it without breaking new parser. |
|
I'll fix this when I have time with something similar to what @shinyquagsire23 suggested. The new shader code is actually broken so I'll be killing two birds with one stone. |
|
I would still prefer to have the syntax checking. Which his shaders are failing at. @dionrhys poke |
|
Should revert shader parser for milestone 1, or at-least have it optional. does not follow the specification and the shaders failing should not be considered a bug. |
|
He fixed atlantica, theres probably other maps which use it though. |
|
Most other custom maps use it, such as Taris RP, Ord mantell RP and so on and they're bugged, so it isn't simply Scizo's maps. It's most maps in JKA. I've tried removing the # from the shader txt file and it hasn't changed anything, it still displays broken textures so it doesn't seem like it's an easily fixed issue, client side that is. |
Not in my experience - I've never come across it in the custom maps I play. It must be common in RP maps, though. If removing the '#' characters didn't fix it, then it's likely a separate issue. |
|
Links to the maps? |
|
Ord Mantell works just fine if you replace the shaders properly. Sounds like you didn't replace it in the pk3 in the right place, or its used in another pk3 that still overrides it. |
|
It's in the shader folder within the pk3 isn't it? That's where I attempted removing it, twice, even redid my entire base folder, still no luck. |
|
I can upload mine later, if you still have issues then. Then you have another pk3 that has priority over it with the shader that is broken still. |
|
https://dl.dropboxusercontent.com/u/13001904/Ord_Mantell_Rp.pk3 It was mantell.shader that had several instances of ######### for this map. I get no issues using that pk3, but I have no other RP maps installed currently. |
|
Still seems like a waste of effort to do that on like a hundred maps when I could either just wait on an eventual fix and acceptance of the ##### in the code and just play regular JAMP until then. |
|
It isn't a comment by any of the parsers old or new, the files are broken 😦 Invalid syntax will be treated as invalid syntax. But the newer parser is flawed in many other ways as it is so we will do something when we get to it. The RP community seems to be the general only area who decided to pass those bad shaders around besides @SzicoVII. |
|
I'm probably going to switch master branch to the Shader parsing that is in rend2. Which will still break on your maps but it at least has warning messages about it being incorrect and then maybe go from there to try and make it workable. |
Apply several commits from ioq3's shader parser / COM_Parse fixes that were still missing. Remove deferLoad variable from ded and vanilla renderer tr_shader.cpp Refs #271. This does not actually fix the issues seen in #271, but it reverts the behaviors of the improved/optimized parser. It still contains syntax checking and would need to be adjusted to treat the number signs differently.
|
Was this ever resolved? If not, I'll just add my input that shinyquagsire23 suggestion seems best as far as how to treat #### based comments. (Console error, but ignore and compile anyway). |
|
It's been reverted to ioquake3 behavior which will actually show the warnings. (As in, its not broken the shaders are) where as the old code that was added to OpenJK was actually broken and having some crash issues as well. At least it should push active mappers to fix their maps. Unfortunately it doesn't really work in such a way that he described. Its not processed or parsed to a point where it can behave that way at this point. As it is now, it does syntax checks to check for the structure of a file so that one bad file does not fuck up the rest in terms of incorrectly placed braces. This is done during the initial load before any of the so-called parsing happens before it puts the whole shader buffer into the mega buffer. FWI other than RP maps, there are very few who this affects. Since he fixed atlantica ages ago. |
…g_buybackTime is set, you will have that many ms to get a full refund on items purchased at the shop when you sell back to the vendor. It does this by adding any purchased items to a vector of pairs called bb_inventory, which stores the item id and the time of purchase. bb_inventory has items removed everytime their time purchased, or the entire list is wiped if the player attacks. Known issues: The way I've hooked it up to ui is extremely janky and needs to be reworked; currently instead of adjusting the displayed price for each inventory item individually based on if the item is in the buyback list, it instead adjusts all the prices if at least one item is on the list. Still looking into why it does that.
This is an issue with Atlantica's shader file because it uses '#' character comment blocks which are not valid in a shader file.
For example:
Here are some screenshots showing how this affects the map:
The retail version of JKA handles this invalid syntax fine somehow by ignoring them, but OpenJK's new shader code chokes on it and doesn't handle the first shader it finds after a #################### line properly. This means that invalid lines will break whatever shader is supposed to come afterwards, and no warnings are given for this happening.
I'd like a discussion on whether the JKA behaviour of ignoring (liberally-parsing) bad syntax should be restored, OR if the new OpenJK behaviour of breaking the parsing could be improved by adding warnings, OR any another solution that someone thinks is better.
cc @SzicoVII
The text was updated successfully, but these errors were encountered: