View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001831 | FSSCP | --------- | public | 2008-11-25 02:55 | 2008-11-30 22:28 |
| Reporter | FUBAR-BDHR | Assigned To | phreak | ||
| Priority | normal | Severity | text | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.6.9 | ||||
| Fixed in Version | 3.6.10 | ||||
| Summary | 0001831: Sending a message with a varible that ends in a number does't display correctly | ||||
| Description | Couldn't figure out how to phrase that one. Here's the scenario. I have to variables named cnt[0] and cnt2[0]. Doing some tweaking I'm attempting to display those variable using send message. Message text is cnt $cnt $cnt2 (and some other variables). Instead of displaying cnt (value of cnt) (value of cnt2) it displays cnt (value of cnt) (value of cnt)2. Looks like it's ignoring 2 as part of the variable name and just sending the 2 as regular text. | ||||
| Additional Information | 3.6.10 build from a couple nights ago. | ||||
| Tags | No tags attached. | ||||
|
2008-11-25 03:17
|
|
|
|
Seems this only happens if 2 or more variables start with the same characters. I just did another test adding a variable named cn[5]. Now $cnt displays 5t and $cnt2 displays 5t2. |
|
|
Is there a way to more clearly define the boundary of a variable, or another way to join them into the string? For instance, in PHP, the equivalent code could be: <?php print "cnt $cnt $cnt2"; print "cnt {$cnt} {$cnt2}"; print "cnt ".$cnt." ".$cnt2; ?> The first is the least defined, the last two both use different methods to clearly define the boundaries of the variable names, one using curly braces and the other using the dot operator to join the string components together. It does sound like the parser is broken, but I'm just curious if there are ways to more specifically define those variables in the string so there's no ambiguity. |
|
|
Fixed in next r4961. Please test using the next nightly and report back. |
|
|
Is the Sexp_variables array already sorted alphabetically? Because then that fix is really unnecessary, it just needs to go through the list in reverse order. |
|
|
Didn't notice, but we'll see if the changes work first. |
|
|
I'm seeing other potential problems with that change. What if the character following the var isn't one of the few you specifically coded for? Wouldn't it be safer to check for valid tokens than invalid ones? If the variable matches, the next character position could just be checked to see if it's a valid character for a variable, letter number or underscore, etc. I think that could be a little cleaner too, but I'm too rusty on my C++ string functions right now to really think how it would work. |
|
|
unfortunately, i do not know the valid characters to use in a sexp variable. |
|
|
Well, it looks like FRED saves the Sexp_variables array alphabetically. It looks like we should just be able to traverse the array backwards and solve the problem, without worrying about going to the EOL or whitespace and then stepping back in the string. Your fix is pretty creative, but I think it makes more sense to just change for (int i=0; i<MAX_SEXP_VARIABLES; i++) { to for (int i=MAX_SEXP_VARIABLES-1; i>=0; i--) { What do you think? |
|
|
Whoa, wait a minute. Phreak, this was well-intentioned, but I don't think you thought it all the way through. We need to be able to specify a variable using just $, because there's no guarantee that a variable will be nicely delineated with spaces or punctuation or whatever. We see this already with $quote and $semicolon. Suppose you have some $quotequoted text$quote that you need to emphasize? The only way to do this is by checking the entire $quote token, without regard to spaces or invalid characters. This fix needs to be reverted. If chief1983's idea works (and it looks like it will work) then we can use that. But we can't use the code you added. |
|
|
I think Chiefs idea will work. The only situation that I can think of that it wouldn't would be something like this: You have 2 variables named say points[27] and pointsleft[1]. You want to have a messages that says 27left $pointsleft would give you 1. Yea it's a bad example. So is there any way to force the variable to end? Say something like $points$left where the second $ cuts it off? Something to make it backward compatible might be to initially do search for a second $ do the compare to the variable list and if it's found use it. If it's not found then proceed with Chief's method. No I haven't actually run into this problem just thinking of what could go wrong. |
|
|
I'm not sure I understand what you're saying, FUBAR, but chief's method will handle both points and pointsleft correctly, using the reverse alphabetical order search. |
|
|
Yes unless you actually want to display the value of variable points with the word left after it (no spaces). Like I said it's a bad example but I can't think of a good one right now. It's probably a 1 in a billion shot anyone would do it but it could happen. |
|
|
ok i've reverted what i have and i'm trying chief's fix out right now. edit: works like a charm |
|
|
Yep just tried it and seems to work fine. That would only leave that one in a billion problem I mentioned. If you don't think that's important then feel free to mark it resolved. |
|
|
Okay cool. FUBAR, the situation you describe should be fixable with judicious variable naming, so it shouldn't be a problem. Marking as fixed. |
|
|
Well I can't see Goob or Phreak on AIM right now, so I just thought I'd point this out. I see the code became: for (int i=MAX_SEXP_VARIABLES; i >= 0; i--) { It looks to me, like this first access will be out of bounds, accessing data that isn't part of of the Sexp_variables array. If it's working already, it may not be a problem after all. That's why my original suggestion was: for (int i=MAX_SEXP_VARIABLES-1; i >= 0; i--) { I'm pretty sure that represents the same range as the original line, before any changes were made to it, except in reverse: for (int i=0; i<MAX_SEXP_VARIABLES; i++) Also, this latest fix hasn't been in a Windows nightly build yet, so if Fubar tested on the 4961 build, it wasn't this fix but the previous one. There will probably be one tonight or tomorrow morning as I am now back in town and my PC will be on again for a bit. If I'm crazy and there's nothing wrong with that then go ahead and close this again, I just wanted it to be doublechecked before it was forgotten. |
|
|
Actually I did test the newer one I just complied it myself. |
|
|
Roger :) |
|
|
Looks like that might be a problem after all: http://www.hard-light.net/forums/index.php/topic,58044.0.html I ran it in debug mode and didn't have an errors but looks like it's not working in this case. I just ran my test mission again with the build from the Knightly builds. No crash from my test mission and no warnings in the fs2_open.log. |
|
|
Sounds like my change needs to be made then. |
|
|
Ooh, good catch by -c-h-i-e-f- Echelon9. Yes, it needs to be MAX_SEXP_VARIABLES-1. |
|
|
Fixed in 4965. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2008-11-25 02:55 | FUBAR-BDHR | New Issue | |
| 2008-11-25 03:17 | FUBAR-BDHR | File Added: var_mess.fs2 | |
| 2008-11-25 03:21 | FUBAR-BDHR | Note Added: 0010265 | |
| 2008-11-25 04:06 | chief1983 | Note Added: 0010266 | |
| 2008-11-25 21:13 | phreak | Status | new => assigned |
| 2008-11-25 21:13 | phreak | Assigned To | => phreak |
| 2008-11-25 22:03 | phreak | Note Added: 0010269 | |
| 2008-11-25 23:22 | chief1983 | Note Added: 0010270 | |
| 2008-11-26 01:24 | phreak | Note Added: 0010271 | |
| 2008-11-26 06:52 | chief1983 | Note Added: 0010272 | |
| 2008-11-26 06:52 | chief1983 | Note Edited: 0010272 | |
| 2008-11-26 15:17 | phreak | Note Added: 0010273 | |
| 2008-11-26 18:00 | chief1983 | Note Added: 0010274 | |
| 2008-11-26 21:36 | Goober5000 | Note Added: 0010277 | |
| 2008-11-26 22:08 | FUBAR-BDHR | Note Added: 0010279 | |
| 2008-11-27 00:20 | Goober5000 | Note Added: 0010280 | |
| 2008-11-27 00:50 | FUBAR-BDHR | Note Added: 0010281 | |
| 2008-11-27 01:17 | phreak | Note Added: 0010282 | |
| 2008-11-27 01:35 | phreak | Note Edited: 0010282 | |
| 2008-11-27 06:11 | FUBAR-BDHR | Note Added: 0010284 | |
| 2008-11-27 07:09 | Goober5000 | Note Added: 0010285 | |
| 2008-11-27 07:09 | Goober5000 | Status | assigned => resolved |
| 2008-11-27 07:09 | Goober5000 | Resolution | open => fixed |
| 2008-11-27 07:09 | Goober5000 | Fixed in Version | => 3.6.10 |
| 2008-11-30 01:50 | chief1983 | Note Added: 0010297 | |
| 2008-11-30 01:51 | chief1983 | Status | resolved => feedback |
| 2008-11-30 01:51 | chief1983 | Note Edited: 0010297 | |
| 2008-11-30 02:31 | FUBAR-BDHR | Note Added: 0010299 | |
| 2008-11-30 02:38 | chief1983 | Note Added: 0010300 | |
| 2008-11-30 04:27 | FUBAR-BDHR | Note Added: 0010302 | |
| 2008-11-30 04:34 | FUBAR-BDHR | Note Edited: 0010302 | |
| 2008-11-30 10:08 | chief1983 | Note Added: 0010305 | |
| 2008-11-30 22:11 | Goober5000 | Note Added: 0010312 | |
| 2008-11-30 22:27 | chief1983 | Note Added: 0010313 | |
| 2008-11-30 22:28 | chief1983 | Status | feedback => resolved |
| 2008-12-21 02:35 | Goober5000 | Note Edited: 0010312 |
