Log for on 12th January 2013:
Times are UTC Toggle Colours
00:47:01  *** Alex3 has joined
01:03:03  *** Alex3 has left
08:52:12  *** Alberth has joined
08:52:12  *** ChanServ sets mode: +v Alberth
09:39:10  *** Zuu has joined
09:39:10  *** ChanServ sets mode: +v Zuu
11:13:12  <Zuu> I've tried to investigate if my fix to FS#5419 could have any bad side effects by allowing negative values where they are not expected, but couldn't find any. However, the string parameter system is somewhat complex.:-)
11:13:15  <Zuu> Fix:
11:13:24  <Zuu> Bug report:
11:14:57  <Zuu> After all, {NUM} and possible more string parameters that take integers, should allow negative values. So allowing GSs to append negative integer parameters to a string doesn't seem to cause any negative side effects.
11:16:38  <Zuu> Do my fix look OK? or do you maybe have a good suggestion for a futher test to perform?
11:18:41  *** fonsinchen has joined
11:18:41  *** ChanServ sets mode: +v fonsinchen
11:27:05  *** frosch123 has joined
11:27:05  *** ChanServ sets mode: +v frosch123
11:34:21  <frosch123> Zuu: are you sure the text is also decoded in dec?
11:35:18  <Zuu> Are you refering to this line:
11:35:19  <Zuu> p += seprintf(p, lastofp, "%X", this->string);
11:35:20  <Zuu> ?
11:35:39  <frosch123> yes, it was hex before, now it is dec
11:36:08  <frosch123> at least stringids are decoded using "param = strtol(s, &p, 16);"; i did not find yet the place where in params are decoded
11:37:09  <Zuu> As I understand that function it creates a string with first the string ID, then each parameter is added with a colon before each parameter.
11:37:51  <Zuu> I haven't fully understood the code which then parse that string.
11:38:04  <frosch123> yeah, but it looks like the decoder always assumes hex
11:38:07  <frosch123> param = strtol(s, &p, 16);
11:38:11  <frosch123> sub_args.SetParam(i++, param);
11:38:28  <frosch123> strings.cpp:870
11:38:40  <frosch123> did you test with numbers > 10?
11:39:37  <Zuu> Numbers to {NUM} or StringIDs?
11:39:51  <frosch123> numbers to num
11:40:30  <frosch123> i am not sure how nested texts work
11:40:40  <frosch123> maybe it is best to encode all NUM and STRING as dec
11:41:22  <Zuu> There is some special handling of GS strings in GetStringWithArgs.
11:41:45  <frosch123> ah, i guess the decoder can also distinguish stringid from num
11:41:50  <frosch123> and still treat stringid as hex
11:42:10  <frosch123> param = strtol(s, &p, 16); <- so i would try to replace this with "param = strtol(s, &p, loopup ? 16 : 10);"
11:42:14  <Zuu> I haven't tried with numbers > 10, but will make a such test.
11:46:58  <Zuu> Passing 15 to {NUM} will display 21 with my "fix", but 15 in trunk (r24892)
11:47:43  <frosch123> try changing strings.cpp:870 to "param = strtol(s, &p, loopup ? 16 : 10);" :)
11:48:54  <Zuu> s/loopup/lookup/ ?
11:49:07  <frosch123> likely :)
11:50:11  <Zuu> That give correct behaviour for -1, 1 and 15 passed to {NUM} from a GS.
11:51:37  <frosch123> do you have a testcase for stringids > 10 ?
11:51:47  <Zuu> The measurement tooltip for rail etc. uses {NUM} and that one seem to correctly display values > 10 (and below too)
11:52:18  <Zuu> Assuming StringIDs start at 0 in GSs, I could just create 11 strings.
11:53:07  <Zuu> Although I would also guss that STR_MEASURE_LENGTH is > 10. But that is not from GS.
11:54:06  <frosch123> SCC_ENCODED is only stuff from gs :)
11:54:27  <frosch123> hmm... oh... what about savegame compatibility?
11:54:46  *** Maedhros has quit IRC
11:54:56  <frosch123> saving a town gui text with trunk and then loading with your patch?
11:55:14  *** Maedhros has joined
11:55:29  <frosch123> i think if that is broken it would be very hard to fix
11:56:38  <Zuu> StringID > 10 from GS appear to work.
11:56:48  <frosch123> also as nested strings?
11:59:00  <Zuu> If I understand it correctly, if you have nested strings, you pass GSText instances as arguments which end up in this->paramt in script_text
12:00:03  <frosch123> siliconvalley has a testcase i believe
12:00:22  <frosch123> the medals are inserted as nested strings
12:00:32  <frosch123> not sure about their stringids
12:07:10  <Zuu> There is a incompatibility when loading a SC game with my patch:
12:07:57  <Zuu> The game was started with r24892 and then loaded into the modified OpenTTD version.
12:08:11  <frosch123> i guess savegame conversion won't work... so maybe there is some way to keep the string encoding compatbilty?
12:08:26  <frosch123> maybe keep it hex and add a "-" in front manually?
12:08:31  <frosch123> (both in encoding and decoding)
12:08:43  <frosch123> hex is also shorter :)
12:09:38  <frosch123> alternatively we can also keep it as hex, and force a uint32 -> int32 conversion on both ends
12:09:49  <frosch123> so -1 is stored as FFFFFFFF in the string
12:10:01  <Zuu> Oh, I see now %X is hex, I read the C++ reference guide to quick and though that it was a int.
12:10:05  <frosch123> what does squirrel use to store variables?
12:10:10  <frosch123> int32, int64?
12:10:18  <Zuu> int -> unsigned int
12:10:47  <Zuu> I'm not sure what the internal integer data type is in squirrel.
12:11:46  <frosch123> int parami[SCRIPT_TEXT_MAX_PARAMETERS]; <- hmm... i believe ottd is set up to make "int" a "int32" ?
12:12:42  <frosch123> yeah, int is int32
12:12:56  <frosch123> so, i guess forcing the int32 -> uint32 conversion on both ends might be the easiest
12:13:52  <frosch123> i.e. "param = (int32)strtoul(s, &p, 16);" and "seprintf(p, lastofp,":%X", (uint32)this->parami[i]);"
12:14:32  <frosch123> any better idea?
12:15:03  <Zuu> Here is the definition of SQInteger which appear to be the internal integer type for squirrel:
12:16:01  <Zuu> Keeping the old encoded format is probably a good solution to maintain savegame compatibility.
12:17:06  <frosch123> hmm, so _SQ64 seems to depend how ottd is compiled
12:17:25  <frosch123> so, for network compatibliity between 32bit and 64bit systems we have to assume 32bit
12:17:48  <frosch123> maybe "parami" should also become "int32" for clarity then
12:19:07  <Zuu> <frosch123> i.e. "param = (int32)strtoul(s, &p, 16);" and "seprintf(p, lastofp,":%X", (uint32)this->parami[i]);" <--- this works both with my test GS and with loading the Silicon Valley game.
12:22:07  <frosch123> "uint64 param;" <- hmm, why does that actually work :o
12:22:10  <Zuu> From what I can see, _SQ64 doesn't appear to be used for any type.
12:26:07  <frosch123> "configure" sets it depending on your host machine
12:26:21  <frosch123> so, nothing that may influence savegame format
12:28:28  <Zuu> but yeah, it is a bit strange that it work to force to int32, while param is uint64.
12:29:43  <frosch123> it likely only works on 64bit or only 32bit
12:29:50  <frosch123> depending on what you tested it on :p
12:30:47  *** Supercheese has quit IRC
12:30:49  <frosch123> hmm, SetDParam also uses uint64
12:31:00  <frosch123> so, every negetive number in ottd uses that conversion
12:31:12  *** fonsinchen has quit IRC
12:31:17  *** Supercheese has joined
12:31:22  <Zuu> I'm on 64bit windows, but 32 bit build using Visual Studio 2008 Express.
12:31:51  <frosch123> well, if SetDParam also does it, it is probably fine
12:31:55  <frosch123> though i do not see why :)
12:35:19  <Zuu> Updated patch with the uint32/int32 solution:
12:36:36  <Zuu> Will it not cause compiler warnings on some systems to pass (int32) to a uint64 variable?
12:37:11  <frosch123> no, only comparison and such
12:37:34  <frosch123> diff looks fine to me
12:46:59  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r24908 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
14:33:56  <frosch123> <- fix the plural description of latvian
14:34:07  <frosch123> and remove some implicit bool->int conversion
14:57:56  <Alberth> swap the latvian description too ?
14:59:15  <frosch123> no idea how to word it; the "other" is never mentioned in the descriptions
14:59:23  <Alberth> an alternative could be to swap the values in code
14:59:43  <Alberth> hmm, would be confusing :p
14:59:46  <frosch123> i wouldn't do that without asking some latvian speaker
14:59:51  <frosch123> maybe this order is the "natural" one
15:00:10  <frosch123> and i do not want to invest the work to get hang of some speaker :)
17:02:52  *** fonsinchen has joined
17:02:52  *** ChanServ sets mode: +v fonsinchen
17:11:55  *** ntoskrnl has joined
17:19:31  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r24909 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
17:20:29  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r24910 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
17:20:43  <planetmaker> has anyone actually looked at FS#5440. The poster has a point
17:21:06  <frosch123> it's on my list, but did not look at it
17:26:52  <Alberth> isn't that why you cannot get bankrupt?
17:27:24  <frosch123> it's more like it always asks for a buyer
17:45:09  *** fonsinchen has quit IRC
18:44:23  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r24911 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
20:34:22  *** ntoskrnl has quit IRC
21:54:32  *** Alberth has left
21:56:51  *** FLHerne has joined
22:10:58  *** Zuu has quit IRC
22:31:25  *** frosch123 has quit IRC

Powered by YARRSTE version: svn-trunk