Time Nick Message 05:47 hecks What would happen if one were to make the server "forget" a dynamically added asset immediately after broadcasting it to existing clients? 05:48 MTDiscord The server would probably let you send it again, but I dont think it would override the texture on already-connected clients 05:48 MTDiscord New clients would have the new texture though 05:49 hecks someone told me they're running a patch like this on their server and I wonder if there's any downsides to this 05:49 hecks they specifically edited the c++ code to remove the asset immediately after broadcast 05:49 hecks so that supposedly it isn't sent to new clients 05:49 MTDiscord Huh, that sounds like a downside to me 05:49 hecks they're using it for temp audio broadcast messages 05:49 hecks so it makes sense but 05:49 MTDiscord Ah, that is a decent usecase 05:49 hecks i wonder if the guy is forgetting something 05:50 hecks if not then maybe this could be a flag for dynamic add 05:50 MTDiscord Ideally youd just make the asset overridable on the serverside 05:50 MTDiscord with some way to clear it too 05:50 MTDiscord Immediate forgetting would then be doable with 2loc 05:50 hecks or just that, make the removal part of the lua API if the server is capable of this 05:51 hecks i'll try to nudge the guy into completing that as a PR if this works 05:51 hecks that would solve one of the big logical problems with dynamic media 05:52 MTDiscord something like minetest.dynamic_add_media("bla"); minetest.dynamic_remove_media("bla"); to achieve what he's already doing 05:52 hecks yeah we don't have a remove yet do we 05:52 MTDiscord We do not 05:52 hecks might be worth just making that 05:52 MTDiscord with just a remove function you can both clear and override media 05:52 hecks the next logical problem to solve would be the broadcast thing; it should be possible to only add media for specific clients 05:52 MTDiscord The only remaining thing is updating the client 05:53 hecks since we already have a callback for a client 05:53 MTDiscord Would it be trivial to make the client dynamically unload media when remove is called? 05:53 hecks hmm 05:53 hecks you mean to forget it in ram? 05:53 hecks not trivial but not difficult i think 05:53 celeron55_ how the server is supposed to work is if you add dynamic media, clients coming afterwards would also get it, but in that case they don't and you have to manually make sure to not make them try to use the media that doesn't exist for them 05:53 hecks just a new packet or packet option 05:53 MTDiscord Whatever works as long as I can call remove and add again to replace an asset on the client 05:54 celeron55_ if it's single use then that isn't a problem 05:54 hecks yeah this is for ephemeral assets 05:54 hecks if you had removal and masking API, the burden would be on the modder to make sure illogical situations don't happen 05:55 hecks that is, that a client is not asked to use media they do not currently have in their set 05:55 hecks and with masking, it should implicitly be possible to send two clients two different versions of an asset 09:33 sfan5 hecks: there's no downside to this, allowing this was planned from the beginning 09:33 hecks oh alright 09:37 sfan5 since 5.5 probably won't be soon I think I can get to implementing the dynamic_add_media v2 API in time 09:54 MTDiscord The server hecks was talking about was likely mine and what I can tell so far is that I can send the exact same file over and over again since I tell clients to not cache it and also delete it from the server cache the moment I send it so older clients will never know about those sent files when they connect 09:55 hecks hi fussel :o 09:56 hecks if this works, would you mind making a PR that makes forgetting media an actual lua API? that shouldn't be hard 09:56 hecks and it would make dynamic media actually viable for things 09:58 MTDiscord Sorry I never learned how to make PRs or use all the git stuff in general but the change was super simple http://sprunge.us/QQW3PK?diff 09:59 MTDiscord But that change also means that this behavior is hardcoded now 10:00 MTDiscord So I opted for entirely removing the ability to tell clients to cache dynamic sent stuff 10:01 MTDiscord sfan5 made that btw ^ 10:03 MTDiscord It being an optional flag for dynamic_add_media() would be way better...also the option to send it to only 1 player and probably even a timestamp of sorts that tells the client to delete it from memory after X seconds 10:04 MTDiscord Or (if possible) right after listening to/viewing it 10:04 sfan5 deleting already used things from memory ha some bigger implicaitons #11531 10:05 ShadowBot https://github.com/minetest/minetest/issues/11531 -- Tile image cache (in RAM) should be reference-counted 10:05 sfan5 not so much a problem for sounds but for textures 10:08 MTDiscord I also wonder if dynamic media gets deleted from RAM the moment you leave the server OR (even worse) only when you exit MT 10:09 MTDiscord I recall there being a memory leak in the past when connecting to multiple servers and only going back to the main menu (it kept certain data in memory) and if this still exists it would be bad 13:30 MTDiscord I just opened #11547. It looks like something I MIGHT be able to try implementing myself, but that depends a lot on how wrong my assumption is that I understand what I'm reading in the code. 13:30 ShadowBot https://github.com/minetest/minetest/issues/11547 -- Screenshot Oversampling 13:31 MTDiscord It came about from a discussion with rubenwardy about how to make high-res screenshot capabilities accessible to everyone in order to meet some demands for screenshot quality on ContentDB, and the multi-tile approach at least seemed promising. 13:32 MTDiscord Anyway, I wanted to open up a public discussion about it rather than taking it on myself, partly because I think it may turn out that I'm not the right person to do this properly. 13:35 MTDiscord What is the etiquette about slapping Supported by Core Dev on a thing that I myself opened? Am I conflict-of-interest-ed out from doing that, or should I just do it since I obviously already support it else I'dn't've filed it in the first place... 13:42 sfan5 latter 15:39 nrz sfan5, you mean if we register a media later it will be pushed to clients ? 15:40 nrz it can be nice, except on android which is pretty slow compared to desktop 15:43 rubenwardy nrz: Minetest has dynamic media as of 5.3 (?), you can push media to clients after loading 16:21 nrz oops ? i'm late 16:21 nrz then what is the new feature ? 16:53 MTDiscord We were talking about a better implementation of it...right now it pushes the media to every supported client and there is also no way to specify whether or not the media should be added to the cache and also no way to tell the clients to delete it after pushing 16:55 MTDiscord I'd love to see loaded media automatically GC'd when not used anymore, especially stuff made using texturemods. It's very easy to accidentally memory-leak clients into oblivion with server mods, even given the sanest possible implementation for the given functionality. 16:56 MTDiscord A good example: a skybox that has a starscape that's not random, but where the stars have specific positions based on predictable factors, i.e. allowing players to use astronomical tools to determine useful information about the world. Interesting gameplay, but murder on the image cache, the way skyboxes need to be drawn right now. 16:56 MTDiscord Also right now it tries to push the media even for 5.2 clients which don't support it so there is no 100% way to actually check whether or not all players received it 16:57 MTDiscord Yeah, you can tell if they did receive it but you'll never know if they will never receive it; they could just be taking arbitrarily long. 16:58 Desour well, you could use minetest.get_player_information to check whether the client supports it 16:58 MTDiscord What I mean is that the callback even gets called for 5.2 clients which cannot ever receive the media 16:59 MTDiscord For my own purposes, I'd be inclined to use [png:, at least for images only, since at least that doesn't necessarily send data to every client immediately ... but that still doesn't solve the memory leak problem either. 16:59 MTDiscord sfan likely forgot that 5.2 already used protocol version 39 16:59 MTDiscord The callback is not supposed to be called for clients that are not capable of receiving and loading the media; that sounds like a bug. 16:59 MTDiscord It should try to send to them but fail to ever call the callback 16:59 Desour oh, the protocol version wasn't bumped? 17:00 MTDiscord Death, taxes, and forgetting to bump protocol version. 17:00 sfan5 I don't remember why it wasn't but that cannot be fixed now or ever 17:00 MTDiscord The protocol version stayed the same for 3 releases 17:00 sfan5 probably because the initial implementation never had the callback 17:00 sfan5 so it wasn't important to know 17:00 Desour imo it makes more sense to also call the callback for older clients. the callback function might do other stuff 17:00 MTDiscord There is no non-idiotic way to tell if clients actually support things. 17:01 MTDiscord The callback could just contain a bool for whether or not the client supports it 17:01 MTDiscord We have opened discussion about how to ensure that modders have a way to determine if clients support things, the conclusion of the discussion was that bumping the protocol version at least once each release was the way to do it, and then we completely ignored that conclusion for all subsequent work. 17:02 MTDiscord Well many pushed for 'minetest.features' but AFAIK you cannot check the features of a connected client 17:02 MTDiscord There are actually a bunch of new features that games with sane releng processes cannot use because of this. 17:03 sfan5 formspec version was bumped last version so you can still tell clients apart IIRC 17:03 MTDiscord minetest.features is a train wreck too. For instance, it can tell you something like whether the entity attachment feature exists but not whether it actually works yet. 17:03 MTDiscord Well, that's a bad example because the answer for that is apparently always false. 17:03 sfan5 what is a better example then? 17:04 MTDiscord but if you need to differentiate between, say, ent attachments that like 50% work and ones that like 90% work, you're SOL. 17:04 MTDiscord This is why mods need to know what release version the client is running. 17:04 MTDiscord If the client can send us a protocol version then they can send us a more fine-grained version and let server mods decide how much of it they can trust. 17:05 MTDiscord We have some funny minetest.features entries like 'pathfinder_works = true' 17:05 MTDiscord Yeah, except that we'd need stuff like entity_attach_works, entity_attach_works_really, entity_attach_works_this_time_i_am_certain, etc. 17:06 MTDiscord That specific subsystem has been broken, fixed, had regressions, been partially fixed, etc. through basically every release since at least 0.4.17, I think. 17:07 MTDiscord Just tell the modders what version the client says it is and let the modders take responsibility for what to do with this information. I don't care if it's via a proto version bump or just an extra get_client_version_string thing. 17:07 MTDiscord I think the main reason devs didn't want to make the exact version available was cause of forks? But other than Multicraft there are hardly any forks that use a completely different versioning scheme 17:08 MTDiscord Forks are irrelevant; that's between the fork maintainer and the modders 17:08 MTDiscord The MT core devs cannot accept responsibility for what forks do or do not do 17:08 MTDiscord if a fork advertises a weird version, or one that collides with an official MT version, then that's between the fork maintainer and any mods that are deceived by that version and thus creates a worse experience for the user. 17:09 sfan5 forks are irrelevant therefore we should make forking as awful as possible? 17:09 MTDiscord I don't see how this makes forking awful 17:10 sfan5 you don't see how forking is made awful when mods check the client version for the most trivial things and probably refuse to work if it looks funny? 17:10 MTDiscord We are not trying to encourage or specifically support forks, that's out of our scope of concern. If forks want to work with MT stuff then they're welcome to support the protocols. 17:10 sfan5 we are not? 17:10 sfan5 regardless this whole thing was already discussed, it's not happening 17:10 MTDiscord Are you saying that we're avoiding allowing mod authors to sanely tell what features the client supports because to do so would be unfair to forks, and it's more important to be fair to forks than to our own modders? 17:11 sfan5 no, you interpreted that 17:11 MTDiscord I suppose when it comes to being fork-friendly, you can't get more friendly than making forks MANDATORY in order to be able to sanely offer clients new features. 17:12 sfan5 being against exposing the client versions which makes forking awful does not imply that I'm against other solutions for this 17:13 MTDiscord The situation as it stands right now is modders need to completely avoid using new features until nobody runs versions of MT that don't support them and can't be differentiated from versions that do, or else they'll get users complaining "X doesn't work" or "Y looks wrong" and be forced to say "you need to upgrade. I'd love to be able to use a fallback or give you a warning about this, but my hands are tied." 17:14 MTDiscord A relatively simple 'solution' might be to send the minetest.features of the clients to servers...I mean it#s just a few bytes no? 17:14 MTDiscord You're against the only known solution. If somebody has a less awful solution then I'm happy to consider it. Since none has been forthcoming this entire time, we should proceed with the least-awful solution, at least considering that it's at least less awful than the non-solution we have. 17:14 sfan5 client versions strings is the only solution? you just named a second one yourself 5 minutes ago 17:15 MTDiscord The cheapest solution was just a mandatory protocol version bump. Essentially just because the wire format hasn't changed doesn't mean the interpretation hasn't changed, which is why we ratified the proto bump option at the time. 17:15 MTDiscord #10933 17:15 ShadowBot https://github.com/minetest/minetest/issues/10933 -- Bump protocol version 17:16 sfan5 "Formspec version was bumped during 5.4.0 (b1ff04e) so identification is already possible and bumping the protocol unnecessary." 17:16 sfan5 oh look what I said 10 minutes ago 17:16 sfan5 looks like this whole issue doesn't exist 17:16 MTDiscord Are we seriously going to have to argue about the policy EVERY release? 17:16 MTDiscord But this is still true for 5.2 and 5.3 then 17:17 MTDiscord I'm not asking for 5.2 or 5.3 or 5.4. I want us to do this for each future release. We need to bump at least SOME thing. 17:17 MTDiscord Or just 5.2* 17:17 MTDiscord It would be very much preferable if we could just bump ONE thing, instead of forcing mods to have to check a complex table of features and versions to figure out whether they're dealing with a particular release before or after a certain PR was merged 17:18 MTDiscord Not being fork-hostile makes sense to me. Unfortunately, not being modder-hostile ALSO makes sense to me so I just can't understand this decision. 17:18 MTDiscord Especially since modders are essentially our intended users. 17:19 sfan5 so telling apart 5.3 and 5.4 wasn't the issue? 17:20 sfan5 weren't you just saying you not being able to do that is a problem? 17:20 MTDiscord I don't know, maybe at the time it was. The underlying problem still remains though, even if 5.4 had a workaround. 17:22 MTDiscord What I need to be able to do is tell with reasonable reliability which release a client is running, or at least what official release equivalent they think they are compatible with. I need a system for doing this that will apply to each future release as it is released. I want to solve this ONCE and then just maintain the solution, rather than have to re-solve the problem each time. 17:22 MTDiscord 5.3 to 5.4 is not the problem, it was an example of the problem 17:22 MTDiscord 5.2 vs 5.3 is also an example of the problem, but as far as I can tell water under the bridge at the point the issue was raised. 17:23 MTDiscord That's fine with me, because as long as all FUTURE versions can be managed, I will eventually end-of-support older versions anyway and differentiating between unsupported versions and other unsupported versions is not crucial to me. 17:24 MTDiscord Each time a new MTE is released, I shift an old version off my support queue. When 5.5 comes out, I will deprecate support for 5.3 in my mods. I'll have to use a hack this time, of checking multiple versions of things, to tell the clients apart, but at least I'll be able to tell. 17:24 MTDiscord When 5.6 comes out though, how can I be sure that I'll be able to tell 5.5 from 5.4? When 5.7 comes out, how will I tell 5.5 from 5.6? 17:25 sfan5 ok so here's the thing which has been bothering the whole time: 17:25 sfan5 writing walls of text here will not make what you want magically come into existence, open an issue so it can be properly discussed 17:25 sfan5 (by more than 2 people that is) 17:26 MTDiscord I thought we already had an issue for this. Ruben's issue was the only one I found so far though, and it looks like he narrowed the scope down to the point where it basically accomplished nothing. 17:27 sfan5 "bump the proto ver each version" was written down by ruben after the first issue 17:27 MTDiscord @rubywarden do you recall if there was another issue thread for the "bump protocol version at least once each release" somewhere? 17:28 sfan5 but the agreement that was there looked more like "let's do something" and not "let's do this specifically to me" 17:28 MTDiscord I'm fine with a "let's do something" approach as long as we can agree on some "something" 17:28 sfan5 you mean #10368 right? 17:28 ShadowBot https://github.com/minetest/minetest/issues/10368 -- Increment formspec/protocol version at every release to indicate client version 17:29 MTDiscord https://github.com/minetest/minetest/issues/10368#issuecomment-687886947 seemed to represent the consensus 17:29 MTDiscord At least it was proposed by ruben and supported by paramat at the time 17:30 sfan5 is that both or one of them? 17:30 sfan5 (the versions) 17:31 MTDiscord Part of the problem might be just that if there exists some "before each release" checklist, this policy change did not get merged into it at the time. If such a thing exists then it seems like we can do a PR for this and go through the normal approval process... 17:31 sfan5 the release checklist is on the wiki, it did get added 17:31 MTDiscord The "bump at least one version" thing doesn't thrill me but I'd consider it acceptable. The "bump at least the protocol version, and maybe also the formspec version" kind of approach I think would make things a lot simpler for modders, but I'm not willing to die on that hill. 17:32 MTDiscord Okay, ugh, the wiki... 17:32 MTDiscord so how safe is it to assume that this will actually be done in 5.5? 17:32 sfan5 it literally was done for 5.4 17:33 sfan5 so I still do not understand why you making such a turmoil about this 17:33 sfan5 ...and considering the consensus is more or less that it would be reasonable to do it for 5.5 too 17:33 MTDiscord It looks like the dev wiki has a "pending discussion" on it, but the discussion was closed on the GH side ... so where is it pending now? 17:33 sfan5 nowher 17:33 sfan5 e 17:34 MTDiscord This bothers me because I thought the problem was fixed but apparently it's actually not. 17:35 MTDiscord It seems like we need to either (1) remove the "pending discussion" thing on the dev wiki, or (2) open the discussion up somewhere. Which one should we do? 17:35 sfan5 1 in any case 17:35 sfan5 I think it's worth quickly discussing what the consensus actually is too 17:36 MTDiscord Here, or on a GH issue? Does it need to be limited to core devs or something (e.g. like this is just a "ratification" process) or open to public comment (which it seems like we've already had quite a lot of though) 17:37 sfan5 here is not gonna work out because there's only you and me here (right now) 17:37 sfan5 there's no formal process so just open an issue, ping cordevs and make clear that one should be agreed upon 17:37 sfan5 perhaps even list thus far proposed options 17:37 MTDiscord open a fresh issue, or reopen the old one where it seems the discussion hadn't fully concluded? 17:38 sfan5 idk, whichever 17:38 MTDiscord Fresh seems like it'd be messy, but at least it'd clear out some of the existing drama... 17:39 MTDiscord I can do a fresh one. Do I just ping like a "core devs" role or something? I'm not very familiar with the hub parts of github... 17:41 sfan5 there's such a role in fact but I'd just ping the people I know who are likely to respond 17:42 MTDiscord Okay, you, ruben, and myself ... should I just pluck out people who responded to one of the issue threads before? 17:42 sfan5 sure 17:42 Krock yes 17:44 MTDiscord Alright, thanks. This sort of meta-issue has been a bit of a mess but hopefully we'll be able to get it all tidied up once and for all. 17:50 Krock is github not working properly? tab size shows as 2, rather than 4 as configured in .editorconfig 17:50 Krock interestingly limited to PRs 17:51 Krock interestingly limited to my PR 17:52 Desour not limited to you. #11538 also suffers this 17:52 ShadowBot https://github.com/minetest/minetest/issues/11538 -- Use utf-8 for the irrlicht clipboard by Desour 17:54 sfan5 when was remote media added? before 5.0 right? 17:54 rubenwardy yeah 17:55 sfan5 thanks 18:00 vrob Tab size used to show up to me as two spaces for the first indent, four afterwards, but now it's two everywhere. 18:01 vrob Never mind, it's only two in PR diffs. Normal files show up as four for me 18:01 Krock will push https://krock-works.uk.to/u/patches/0001-Fix-inconsistent-integer-comparison-warnings.patch in 10 minutes 18:01 vrob Odd, all the same 18:01 Krock unless someone has more of them to fix 18:02 Krock yes. broken github. 18:02 vrob Crud, adding `?ts=2` to the URL makes tabs ONE space 18:04 sfan5 Krock: lgtm 18:04 sfan5 consider checking CI output to see if there are more 18:04 sfan5 but I don't think there were 18:08 Krock there's a bunch of SRP false-positives 18:10 Krock pushing 18:10 Krock done 18:13 sfan5 Krock: for #11548: that's not the problem at all, the client does this right 18:13 ShadowBot https://github.com/minetest/minetest/issues/11548 -- HUD: Do not assume int values on unknown HUD types by SmallJoker 18:14 sfan5 https://github.com/minetest/minetest/blob/6e8aebf4327ed43ade17cbe8e6cf652edc099651/src/script/common/c_content.cpp#L1999-L2000 18:14 sfan5 the server will fall back to the default type (number) if it gets an unknown string passed 18:15 sfan5 (server = 5.4.0, client = whatever version) 18:16 Krock oh! 18:17 Krock that code would be for unknown server-side values 18:19 Krock isn't it also a problem when an additional field is added? 18:19 Krock hmm 18:19 sfan5 the code that processes the client even ends up in a per-type switch IIRC 18:19 sfan5 so that should be safe 18:20 Krock right. game.cpp 18:23 Krock what's expected in the error case? warning? silent ignore? 18:24 Krock unique warning perhaps 18:24 sfan5 warn would be useful imo 19:14 Krock alright. PR updated. 19:14 Krock should be OK (TM) 20:40 sfan5 rejoice, I decided to work on dynamic_add_media v2 today and finished it 20:41 rubenwardy :O 20:42 MTDiscord PR when 20:52 sfan5 now 20:52 sfan5 took about 10 hours minus lunch, dinner and other short downtimes 20:54 MTDiscord If I enable ephemeral, can I send the same file twice? 20:55 sfan5 no 20:55 sfan5 the opposite, this is only allowed with to_player 20:55 MTDiscord "this" being, sending the same file twice? 20:55 sfan5 yes 20:55 sfan5 I assume you mean sending the same file with the same name and contents to different players 20:56 MTDiscord Well, mostly same name 20:56 MTDiscord For instance, to update an existing texture 20:56 sfan5 a file with the same name can never be sent to the same client twice (this includes media sent at startup) 20:57 sfan5 if you have some way to use a different texture (like on an entity) there's no restrictions 20:58 MTDiscord so to get the same result, id enable ephemeral and just increment the name whenever I want to update it? 20:58 sfan5 yes 20:58 sfan5 that works whether you set ephermal or not 20:59 MTDiscord @Warr1024 this solves the memory issue with your dynamic skybox, yes? (once you can drop support for earlier versions) 20:59 sfan5 though it's obviously a good idea as you won't ever be needing the thing again 20:59 sfan5 well nah 20:59 sfan5 the client still keeps all textures it has ever received in memory 20:59 sfan5 #11531 for that 20:59 ShadowBot https://github.com/minetest/minetest/issues/11531 -- Tile image cache (in RAM) should be reference-counted 20:59 MTDiscord Meh, true 21:00 rubenwardy so, will the only difference on old clients be that they still store the texture in the cache folder 21:00 MTDiscord This still at least reduces the memory usage for Warr's dynamic skybox 21:00 MTDiscord Since he has skyboxes per-player 21:01 MTDiscord So now he wont have to send every player updated skyboxes that they dont need 21:01 sfan5 rubenwardy: elaborate 21:02 MTDiscord Oh, and its also helpful that we have minetest.encode_png now, that will make using this much easier 21:03 MTDiscord I'm using texturemods right now, and those are generated per-player on the client side anyway, so I'm already fine. 21:03 MTDiscord Ahh, true 21:03 MTDiscord My bad 21:03 MTDiscord I expect that texturemods would be the way I'd do it anyway, so really it's only the GC that I'd need. 21:04 MTDiscord I'm not sure if I'd have a use-case for pre-baking images as a png server-side or anything in this case. 21:04 rubenwardy Presumably `to_player` will work with 5.4.0 clients, as the server can just not tell the client about the texture. Would the only feature difference be that 5.4.0 clients will still store in cache/? 21:04 sfan5 non-caching will also work on 5.4 21:04 MTDiscord How? 21:04 rubenwardy forwards compat?! :O 21:05 MTDiscord I could imagine myself using dynamic media for sounds, but I'd kinda rather have "audio texturemods" anyway :-D 21:05 sfan5 the 5.4 code is forward compatible by including a boolean in the packet indicating cache state, the 5.4 server only ever sends true 21:06 MTDiscord Huh, interesting. Y'all practically planned for this. 21:06 MTDiscord (Well, chances are you actually did) 23:28 MTDiscord This is exactly what I'm using right now on my 5.4 server to prevent any kind of client caching...I set that bool to false 23:29 MTDiscord And this allows me sending the exact same file infinite times too 23:30 MTDiscord Which is wasteful right now of course but it works