-
Notifications
You must be signed in to change notification settings - Fork 18
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
Enable importing .ksp files from folders (#400) broke some imports here (possible old math library related) #447
Comments
Hmmm, I have some code that imports that same file in exactly the same way and it all works fine still... Weird! |
Wow, really? Hmm. It's obviously easy enough just to use the right commit when I need to get back to that code, but I figured a lot of people are maintaining code with the same library. I did try a bunch of path simplifying to make sure it wasn't that. This was all on macOS. Will take another look when I have some time. |
Can you try with the very latest commit, please? Unsure if it'd fix but worth a shot I guess... |
Unfortunately, still an issue with the latest commit here. When I find some time I will see about debugging it here as well. |
@dxmachina Would you mind DMing me the exact code which reproduces this issue? As minimal as possible. Thanks! |
@mkruselj I'll take a look again when back after the 1st, but if I recall it was only a matter of importing the library as above. Don't think I even needed to call a function to run into the error on compilation |
Right. That's weird, because that exact math lib version imports just fine over here... Happy new year! |
Yep, going to have to dig in a bit more here to figure out the actual issue. I was able to import into a new file and (after remembering how this thing worked with the math modes) call some of the functions without issue. So we're seeing the same behaviour. BUT, I do have a number of legacy projects that made very extensive use of this library that still won't compile without using older commits, so it may be more related to a specific function, or perhaps some kind of issue with nested macros/functions. Will get you a proper simple repro once I sort it out. NBD though... easy enough to swap out with an older commit to compile these when necessary. Happy New Year to you as well! |
You shouldn't need to faff around with swapping out commits just to make older stuff work, so yeah, more exploratory digging to find the root cause would be really welcome... |
@mkruselj Ok, I figured this out this morning. It looks like the version of the math library in all my problem repos have Windows style line endings (I'm on Mac now). I double checked and all these copies of KSPMathV702.ksp have been unchanged in the history of these repos, so best guess is after a compiler change it is no longer handling these and they look like a ton of extra blank lines. In effect, I think the error is happening on any lines like this:
Which due to line endings are being seen as:
So fixed line endings and everything looks good. Not sure anything needs to be done on the compiler side, but I think that's it. |
Curious. Good to know. But if I remember correctly the compiler actually has a function that is executed on file load that is supposed to fix the line endings. However this was apparently not working for the past several versions due to the incorrect way I was monitoring the plugin's load state. This should be fixed as of commit 5af048a, so worth checking your math lib file before you fixed line endings if it works from that commit onward... |
@dxmachina Just a reminder ping if you could verify the above? Thanks! |
@dxmachina And another ping, I don't want to close this issue without having the above confirmed. Thanks! |
Apologies for the delay @mkruselj That commit did not resolve it for me using the previous state of the math library but I haven’t spent any time further debugging after fixing the line endings. I’ll try to get you more info on it soon if you want to leave open. |
Yeah I do want to resolve it, if at all possible (might also help to have that old version of the math lib with messed up EOLs, too!). If EOL conversion happened, you should see a message in Sublime Text's console during file open, just FYI. |
@dxmachina OK, I think my prior attempt at a fix for detecting the Sublime plugin loaded state was wrong. I noticed this when I was pasting valid KSP code in an empty untitled file, and syntax coloring would not automatically update (as it was expected) - even in this latest 1.22.0 release. Turns out, I completely did not understand what is going on with global variables in Python. But now I think I do. Can you please check one more time with commit 188912d? I would hope that the automatic fix for mixed EOLs in a file should kick in now! I'd still love to have that .ksp file with mixed line endings, though... (If it were just Windows style line endings consistently throughout the file, it would not be a problem, because Sublime Text is automatically normalizing EOLs, so opening a Windows file on macOS would work just fine and would not create extra newlines...) EDIT: Found the original download of the math library. I see the issue now! And 188912d doesn't fix it. But I now actually know what's going on, and I think I can fix it! |
@dxmachina I would love to have your confirmation if the currently opened PR fixes this issue on your end as well. I have the original download of math library working now but having it double checked is always preferred. I don't want to merge the PR without the confirmation. Thanks! |
@mkruselj Yes! You've done it, I believe all my old repos compile now without any manual intervention. Thanks for staying on it. Relatively obscure issue but probably does affect a fair amount of older projects. |
This commit specifically broke code that I'm maintaining here, possibly only related to the import of the old Big Bob math library. 63a497f
The line is simply:
import "KSPMathV702.ksp"
Yields: Syntax error! (line 2279)
../common/external/KSPMathV702.ksp:4625
Release 1.17.0 is fine
The text was updated successfully, but these errors were encountered: