--- Log opened Tue Jan 15 00:00:41 2019 20190115 00:40:17-!- vn971 [~vasilii@2a02:a210:2203:c000:7f64:7780:4177:6131] has quit [Ping timeout: 252 seconds] 20190115 01:00:49<+wesdiscordbot> vn971, for afterlife and laterlife, give to the AI allow_observer=no, because then observers won't see the fogged vision of the AI, but the one of the players in these turns 20190115 01:21:13-!- vn971 [~vasilii@ip-213-127-111-47.ip.prioritytelecom.net] has joined #wesnoth-umc-dev 20190115 01:37:20-!- vn971 [~vasilii@ip-213-127-111-47.ip.prioritytelecom.net] has quit [Ping timeout: 268 seconds] 20190115 01:40:57-!- vn971 [~vasilii@2a02:a210:2203:c000:7f64:7780:4177:6131] has joined #wesnoth-umc-dev 20190115 02:35:08-!- celmin|away is now known as celticminstre 20190115 02:35:14-!- celticminstre is now known as celticminstrel 20190115 04:16:47-!- celticminstrel is now known as celmin|sleep 20190115 06:56:24-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has joined #wesnoth-umc-dev 20190115 08:47:21-!- vn971 [~vasilii@2a02:a210:2203:c000:7f64:7780:4177:6131] has quit [Quit: Leaving.] 20190115 08:53:56-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has quit [Ping timeout: 268 seconds] 20190115 09:42:01-!- vn971 [~vasilii@office.zivver.org] has joined #wesnoth-umc-dev 20190115 10:00:49<+wesdiscordbot> @josteph Is this the same kind of bug? 20190115 10:00:50<+wesdiscordbot> https://cdn.discordapp.com/attachments/442775044590927873/534673303911137280/wesnoth-20190115-103755-8564.log 20190115 10:07:18-!- vn971_ [~quassel@2a02:7aa0:1619::bac5:9483] has joined #wesnoth-umc-dev 20190115 10:07:37-!- vn971_ [~quassel@2a02:7aa0:1619::bac5:9483] has quit [Client Quit] 20190115 10:07:56-!- vn971_ [~quassel@2a02:7aa0:1619::bac5:9483] has joined #wesnoth-umc-dev 20190115 10:11:28< Soliton> yes. 20190115 10:13:12< Soliton> looks like you need to get rid of about 5 textdomains. 20190115 10:18:16-!- vn971_ [~quassel@2a02:7aa0:1619::bac5:9483] has quit [Quit: https://quassel-irc.org - Chat comfortably. Anywhere.] 20190115 10:18:28-!- vn971 [~vasilii@office.zivver.org] has quit [Quit: Leaving.] 20190115 10:18:48-!- vn971 [~quassel@2a02:7aa0:1619::bac5:9483] has joined #wesnoth-umc-dev 20190115 11:40:15<+wesdiscordbot> How did you come up with that number? 20190115 11:41:32<+wesdiscordbot> And 1.14.6 (with the supposed fix) is coming soon, so it's not that grave a problem. Especially since 'only' some descriptions are affected, instead of a whole scenario/campaign. 20190115 12:42:13< Soliton> the numbers in those error messages go up to 0x84 and the issue starts at 0x80. 20190115 12:43:50<+wesdiscordbot> there's some more info here https://forums.wesnoth.org/viewtopic.php?f=4&t=49263#p637264 20190115 12:46:50<+wesdiscordbot> You mention "The error was platform-dependent (it would only occur on platforms whose char type is signed)", but I'd like to add that char is signed on every major platform. 20190115 12:47:03<+wesdiscordbot> https://stackoverflow.com/a/2054959 20190115 12:47:16<+wesdiscordbot> "For gcc, the default is signed" 20190115 12:47:40<+wesdiscordbot> And I'd assume it to be the same way for Clang as well, given that it tries hard to be compatible with GCC. 20190115 12:47:58<+wesdiscordbot> @jyrkive Thanks, that's a good tip. (and explains why I could reproduce the issue on linux) 20190115 12:48:11< Soliton> so Konrad2 is just the first brave wesnoth user to install that many add-ons. :-) 20190115 12:48:23< Soliton> or the first to report it anyway. 20190115 12:48:24<+wesdiscordbot> I usually stick to the portable subset of the language (wherein char is of implementation-defined signedness) 20190115 12:48:47<+wesdiscordbot> @jyrkive I don't suppose you happen to have a wesnoth.exe from git that you could give to Konrad for him to confirm the fix? 20190115 12:49:10<+wesdiscordbot> Indeed, we shouldn't depend on platform-specific stuff (except for really universal things such as IEEE 854). 20190115 12:49:59<+wesdiscordbot> No, not really. I can compile Wesnoth on my PC, but I use Visual Studio, and VS builds aren't ABI compatible with the DLLs which official builds use. 20190115 12:50:50< Soliton> looks like the jenkins windows build is still broken. 20190115 12:52:02<+wesdiscordbot> Hmm. The code still does a conversion from char to unsigned char and back. I assume that will work in practice, even if it may not be 100% correct de jure. 20190115 12:52:39<+wesdiscordbot> Thanks (re wesnoth.exe) 20190115 12:54:27<+wesdiscordbot> (updated the forum post) 20190115 13:00:30<+wesdiscordbot> From what I can tell, the conversions in that line (after this commit https://github.com/wesnoth/wesnoth/commit/520b2c9d59ceec90dc7ec4dea109e68834a44f55 ) are as follows: 1. string_[begin + 2]: char -> unsigned char (explicit cast) 2. unsigned char -> int (in order to multiply by 256) 3. string_[begin + 1]: char -> unsigned char (explicit cast) 4. unsigned char -> int (in order to perform an add with the int on the 20190115 13:00:31<+wesdiscordbot> right side) 20190115 13:00:58<+wesdiscordbot> So both of them go char -> unsigned char -> int. There isn't a conversion back to char anywhere. 20190115 13:05:45<+wesdiscordbot> Soliton: Just having installed that many add-ons isn't enough. You also have to play one of the (comparatively few) add-ons affected. And that might take a while even if or rather because you have them all installed. ^^ 20190115 13:15:28-!- celmin|sleep is now known as celmin|away 20190115 13:16:42<+wesdiscordbot> @Konrad2 Yeah, you have to play one of the last 5 addons in the order of textdomain id's. 20190115 13:18:09<+wesdiscordbot> @jyrkive Sorry, I got it the wrong way around. The t_string_base constructors do a cast from unsigned int to char, then that line does a cast back from char to unsigned char (and then to int for multiplication and to unsigned int for assigning to id). 20190115 13:19:01<+wesdiscordbot> https://github.com/wesnoth/wesnoth/blob/fd72c291468af857ade48f8b88625041a6b5ceb5/src/tstring.cpp#L278-L280 20190115 13:19:11<+wesdiscordbot> https://github.com/wesnoth/wesnoth/blob/fd72c291468af857ade48f8b88625041a6b5ceb5/src/tstring.cpp#L118 20190115 13:20:28< celmin|away> Shouldn't this just be reverted https://github.com/wesnoth/wesnoth/commit/520b2c9d59ceec90dc7ec4dea109e68834a44f55 20190115 13:20:53< celmin|away> Because that's multiplying a value of type char by 256 isn't it? 20190115 13:21:11<+wesdiscordbot> celmin, with inside the cast it's plain wrong. 20190115 13:21:12< celmin|away> (Unsigned char but still) 20190115 13:21:24< celmin|away> Hm? How so? 20190115 13:21:59< celmin|away> It doesn't seem wrong to me? 20190115 13:22:04<+wesdiscordbot> (unsigned int)(char)(unsigned int)0x80 != 0x80 20190115 13:22:13<+wesdiscordbot> (unsigned char)(char)(unsigned int)0x80 == 0x80 20190115 13:22:22< celmin|away> ??? 20190115 13:22:29<+wesdiscordbot> @josteph Narrowing unsigned-to-signed conversion is implementation defined, so we indeed have a potential problem in lines 278-279. 20190115 13:22:55<+wesdiscordbot> And as jyrkive said, multiplying a char by 256 is fine. The 256 literal is an int so the char constant is promoted to a larger type for the multiplication. 20190115 13:23:11< celmin|away> Where are you getting that sequence of casts from... 20190115 13:23:27<+wesdiscordbot> celmin, the two github URLs I pasted just above 20190115 13:23:29< celmin|away> Ah, so integral promotion does kick in, I was wondering if that might be the case. 20190115 13:23:56<+wesdiscordbot> @jyrkive Would it suffice to change those lines to cast to unsigned char? 20190115 13:24:00< celmin|away> Your second link is the same as my first link (wrt the code displayed) 20190115 13:24:03<+wesdiscordbot> (Well, those lines and the copy of them in the other overload) 20190115 13:24:12<+wesdiscordbot> they have different anchors 20190115 13:24:17<+wesdiscordbot> one is line 118 20190115 13:24:24<+wesdiscordbot> one is lines 278 to 280 20190115 13:24:45< celmin|away> I was talking about 118 20190115 13:24:53<+wesdiscordbot> @josteph No. The following unsigned-to-signed char conversion would also be implementation defined. 20190115 13:25:10<+wesdiscordbot> celmin, The sequence of casts includes both 278 and 118. 20190115 13:25:19< celmin|away> Oh, okay. 20190115 13:25:51< celmin|away> ...are you assuming 118 happens first or 278 happens first. 20190115 13:25:58<+wesdiscordbot> 278 first 20190115 13:26:02< celmin|away> Right. 20190115 13:26:06< celmin|away> That's what I thought. 20190115 13:26:20<+wesdiscordbot> id is unsigned, id & 0xff is unsigned, gets cast to char, assigned to a char 20190115 13:26:31<+wesdiscordbot> then on 118, the char is cast to unsigned char, promoted to int, assigned to unsigned int 20190115 13:27:19<+wesdiscordbot> @jyrkive I assume a non-narrowing conversion would be less liable to have unintended results, but in any case, what would you recommend? 20190115 13:27:24< celmin|away> Didn't this originally use reinterpret_cast? Or am I thinking of a different location in this file? 20190115 13:27:27<+wesdiscordbot> Other than "don't encode ints into a string in the first place" 20190115 13:28:05<+wesdiscordbot> For now, I'd just keep it this way. Encoding integers into strings is difficult to do in a by-the-book way. 20190115 13:28:09<+wesdiscordbot> celmin, "this" being which one ? 20190115 13:28:19< celmin|away> Uh... not sure... 20190115 13:28:27<+wesdiscordbot> @jyrkive Right. Thanks for the review! 20190115 13:28:54< celmin|away> I definitely recall writing some code in this vicinity that used unions, and IIRC I later changed it to reinterpret_cast because someone complained... 20190115 13:29:37<+wesdiscordbot> Heh. In the random number generator, I changed reinterpret_cast to an union because GCC complained. 😛 20190115 13:29:54< celmin|away> Heh. 20190115 13:30:08<+wesdiscordbot> gcc complained about the RNG? I didn't know gcc played wesnoth 20190115 13:30:13< celmin|away> XD 20190115 13:30:26<+wesdiscordbot> It gave a compiler warning about violating strict aliasing rules. 20190115 13:31:41<+wesdiscordbot> (Overly aggressive warning if you ask me. GCC knows perfectly that two pointers overlap when I use reinterpret_cast like that and can easily just disable strict aliasing optimizations.) 20190115 13:31:44< celmin|away> What does that mean again? 20190115 13:32:11< celmin|away> Strict aliasing means pointers overlapping? 20190115 13:32:21<+wesdiscordbot> https://blog.regehr.org/archives/1307 20190115 13:33:11< celmin|away> Ahh. 20190115 15:12:53-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has joined #wesnoth-umc-dev 20190115 17:43:07-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has quit [] 20190115 17:47:02-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has joined #wesnoth-umc-dev 20190115 19:23:11-!- Ravana [~Ravana@wesnoth/mp-mod/ravana] has joined #wesnoth-umc-dev 20190115 22:37:33-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has quit [Ping timeout: 245 seconds] --- Log closed Wed Jan 16 00:00:42 2019