Time Nick Message 19:00 Krock might I merge #14045 before release or is it better to wait? 19:00 ShadowBot https://github.com/minetest/minetest/issues/14045 -- Server: avoid re-use of recent ParticleSpawner and Sound IDs by SmallJoker 19:03 MTDiscord Krock: it would be nice to implement Wuzzy's suggestion of enabling a feature flag so that I don't have to use some ugly heuristic to detect the corrected IDs... 19:05 Krock use minetest.get_version().proto_max < 43 19:06 MTDiscord I suppose if I only support release versions and not dev versions that could work 19:06 Krock I suppose people who use dev versions will continue to use (newer) dev versions too 19:06 Krock same for stable version users 19:07 MTDiscord Not necessarily, for public servers. I tend to try to stick to release, until a critical bug gets fixed, and then I stick to the oldest version of MT that fixes all known critical bugs until the next release. 19:09 Krock wouldn't that become obsolete with the release? 19:09 Krock since you tend to stick to release (if available) 19:10 MTDiscord I tend to have to support the latest release, the previous release, AND the latest release plus critical patches, in practice. Usually it's not much of a problem to support dev versions after the latest release, at least. 19:11 Krock would your workarounds introduce side-effects on "in-between" versions? 19:12 Krock nvm. the bump already happened 19:13 MTDiscord Either way I can't disambiguate them. With the workaround, it corrects the old behavior, and doesn't break the new behavior, but it DOES introduce a memory leak with the new behavior. I can heuristically detect that by noticing that enough new IDs are allocated but none are reused, but then that makes the whole thing a fair bit messier than it is now. 19:14 MTDiscord Also that heuristic would fail if somehow somebody DID actually allocate enough sounds or particles or something all at the same time before any of the old ones actually expired. 19:14 Krock local needs_workaround = minetest.get_version().proto_max < 43 or minetest.get_version().proto_max == 43 and minetest.get_version().is_dev 8) 19:15 Krock it's not the most beautiful way to do it and still does not cover everything. I suppose you'd like to have a feature flag that would then stick around in the API until eternity 19:15 MTDiscord Maybe my best bet is to just only support the workaround for 5.7.0- and assume that all 5.8.0 things are corrected, and then I'll just miss a few dev versions in the middle. 19:16 MTDiscord Heh, well, Wuzzy would seem to want to have feature flags for everything... 19:16 MTDiscord We might as well have a feature flag for each commit, since it's hard to tell when a bug actually gets fixed, or a regression introduced ๐Ÿ˜‘ 19:17 MTDiscord Quick question, what is #14016 waiting on? 19:17 ShadowBot https://github.com/minetest/minetest/issues/14016 -- [no squash] Return texture filter settings to previous state by sfan5 19:17 Krock we wouldn't have this issue if we'd use SVN with auto-increment version numbers :P 19:17 MTDiscord Before the protocol version bump it was hard to tell if entity attachments were actually working, for example, because the feature flag only covered when the promise for the feature was added, and not the like 3 or 4 times when it was kinda fixed but not quite entirely. 19:17 Krock @greenxenit on someone who can reproduce the bug and confirm the fix because I didn't notice anything 19:17 MTDiscord You mean somebody other than me? 19:18 MTDiscord I'm not sure how many confirmations you need. Do you need a core dev to actually confirm it, or just to approve it based on somebody else's confirmation? 19:18 Krock @warr1024 possibly in the last meeting I wondered whether your "LGTM" means "looks good in game" or "code looks good" 19:19 Krock so I could not figure out whether the bug is actually reproduced and confirmed as fixed by this PR 19:19 MTDiscord It was intended as a direct response to "please test" above it, implying that it tested correctly for me. 19:20 MTDiscord I assume that code-review-wise, sfan5 and grorp had it covered. 19:20 Krock okay I see. thank you for clarifying. Indeed, the code check happened in the other review. 19:21 MTDiscord If you wanted clarification on what exactly my comment meant, you probably could have commented as such in the GH thread and I'd have seen that, and may have been able to get you that clarification earlier. 19:22 MTDiscord Thankfully this is one of those rare occassions when I am actually logged into GH so there, added the clarification to the comment ๐Ÿ˜„ 19:22 MTDiscord So.... good to go? 19:23 Krock right. I just checked the IRC logs and I did indeed miss the opportunity to actually ping you in any way. Sorry for that. Considering that it had little attention in the meeting, it might be merged soon regardless 19:24 MTDiscord Was this the last blocker? 19:24 Krock I assume this didn't ping on Discord side https://irc.minetest.net/minetest-dev/2023-11-26#i_6135400 19:25 MTDiscord Correct 19:25 Warr1024 @Warr1024 test 19:25 MTDiscord Okay, with the @ prefix it works 19:25 MTDiscord Also correct :^) 19:25 Krock good to know :) 19:26 MTDiscord I have (as possibly other dual-discord-IRC users may have) disabled pings on the IRC side otherwise MTDiscord pings the hell out of me, so pinging me via the Discord way is most reliable. 19:26 MTDiscord (your mention @ me earlier would have worked perfectly, had the H not been missing ;p) 19:27 MTDiscord So what strings have to be pulled to get 5.8.0 released before December 19:27 Krock pull an entire harp 19:28 MTDiscord If 5.8 isn't released by December that could put the game jam in a bit of a jam ... meta-jam, I suppose. 19:29 MTDiscord Well it looks like 14016 can be merged any time, which should have been the last remaining PR 19:29 MTDiscord Unless 14045 is now a thing 19:30 MTDiscord I'd rather not wait for 14045. 5.8 will be no more broken than any previous release was, without it. 19:30 MTDiscord :evaporate: 19:31 MTDiscord Well it effectively has 2 approvals so 19:31 MTDiscord If it doesn't require waiting then I'm okay with merging, I just don't want to wait on it. 19:32 MTDiscord Krock: Are you waiting for a core dev opinion on merging 14045 now? 19:33 Krock greenxenith: that was the idea. yes. Since that's the final instance to determine whether a PR is sane to be merged at all, considering the close 5.8.0 release date. 19:33 MTDiscord @celeron55 ^ 19:35 Krock ๐Ÿคจ 19:35 MTDiscord (he is online on discord) 19:38 MTDiscord Whether or not he is actually present or will respond is a different story, but im willing to take that bet 19:39 celeron55 i'm not sure what's being asked of me 19:40 MTDiscord > might [Krock] merge #14045 before release or is it better to wait? 19:40 ShadowBot https://github.com/minetest/minetest/issues/14045 -- Server: avoid re-use of recent ParticleSpawner and Sound IDs by SmallJoker 19:40 ROllerozxa what's with all the hurry to get a release out before the jam starts anyways? 19:40 ROllerozxa just tell people to develop against 5.8.0-rc1 to begin with, and then move onto 5.8.0 stable when it releases 19:40 celeron55 14045 seems like quite the regular variant of jank. not having is not a showstopper. the risk either way is not massive, how long will it take to test it enough for release? 19:41 celeron55 not having it* 19:41 MTDiscord ROllerozxa: We don't change targets mid-jam. We target stable versions. 19:43 celeron55 i think Krock as the author is in the best position to guesstimate the risk and the amount of testing required 19:46 Krock there's not much that can go wrong, which is primarily why I asked for another opinion. anyway. merging #14016 for now. 19:46 ShadowBot https://github.com/minetest/minetest/issues/14016 -- [no squash] Return texture filter settings to previous state by sfan5 19:46 sfan5 IMO merging it is fine 19:47 celeron55 it seems harmless. but doing a code review and stating these exact words are how the worst bugs are made 19:47 Krock alright thanks. so I'll also queue 14045 and merge the two in say 20 minutes for any objections. 19:48 Krock celeron55: even testing does not exclude all bugs. it always depends on the complexity 19:49 Krock for example the new inventory mouse movement features basically broke the handling on Android 19:49 celeron55 aren't activeobject ids generated in roughly this way? i.e. trying to assign specifically an older id 19:49 Krock exactly. the code is in fact almost copied over 19:50 celeron55 well at least the strategy is proven then. as long as the implementation works it should be fine 19:51 Krock ๐Ÿ‘ 20:08 Krock merging (2) 20:10 Krock done 20:12 MTDiscord nice 20:12 MTDiscord did we already have the proto version bump for 5.8 then? 20:13 Krock yes https://github.com/minetest/minetest/blob/master/src/network/networkprotocol.h#L218-L224 20:15 MTDiscord oh, right, nice 20:51 [MTMatrix] Release within tomorrow? :D 20:52 MTDiscord ah btw. it's kinda "low-priority" (at least on x86) but i suspect all our debian / ubuntu builds (e.g. on launchpad) still use the old LuaJIT (because that's what's packaged there). 21:33 MTDiscord https://github.com/minetest/minetest/commit/a7e545609909e4e56b60878b2030fa13ddc630de#diff-6ee8aa777b5cee78a2931426af30b31cfa5e80cbb033ff76cf80f5dfcd877bdeR1640-R1643 this looks wrong. It should increment free_id before, or it will fail after the first try. 22:50 sfan5 btw irr#255 should also be included in the release 22:50 ShadowBot https://github.com/minetest/irrlicht/issues/255 -- Android: Make ALooper_pollAll call always non-blocking by srifqi