New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UDP query of game script #9234
UDP query of game script #9234
Conversation
Reopened from old flyspray issue OpenTTD#6211. Patch adds a possibility to query OpenTTD server for running game scripts and returns its name and version if any present. Goal of this fork is to make also ingame interface.
Honestly, I am not really sure what to do with this. You mention that you want to reopen an idea, but you don't start a discussion; you just show some code. You really have to explain a bit more. The Pull Request template we have helps you with that. It tells you to write a motivation and a description of your Pull Request. But as you didn't do that, you didn't "reopen" anything really. You just showed us some code. Please help us help you. Don't make it our work to figure out what your intentions are, what you want to accomplish, etc. That really is your job: convince us why we should want this. And for that we have a template, to guide you through that process. Could you please fill in the template and help us out what is the idea here? Thank you! |
I have probably missed the template, my bad. This patch focuses to extend fetching game info over UDP from multiplayer servers. Currently you can get basic game information and NewGRF details, but nothing about game scripts. I think it would be useful to have possibility to found out, if server is running game scripts. When servers has a game scripts, it and could be shown in ingame lobby window and could be added also to web server listing . Currently this patch solves it by adding new packet type, so there is extra UDP communication needed, but I think it would be better to rewrite it a little bit and incresase NETWORK_GAME_INFO_VERSION and add it to the end of the PACKET_SERVER_GAME_INFO packet in SerializeNetworkGameInfo() in game_info.cpp. |
What I miss in your description, is a motivation. What do you want to accomplish with this? I can see it adds a packet that gives some GameScript information. But why specific that information? And why that alone? In other words: how do you intend to use it? What is the request that made you build this? Just that you find it useful is not really a convincing argument :) Please elaborate a bit more :) Tnx! (edit: at least, I assume you didn't add a full packet just for the game script name; that could be done in the GameInfo too. Given the amount of data, I assume you have a higher purpose here. But this is my interpretation, so a bit more explaining how you want to use this would really help) |
I have two motivations:
|
Tnx! In that case this PR needs a bit more work of course :) Starters, I would add this to GameInfo. It is just a simple string, possibly with a version, so that would make two simple strings, or maybe merge it into a single one ("name version" or something). It means we don't need a new packet, which is always a bonus. But mostly, this PR would also need to implement the usage of this packet. We tend not to like stuff that just adds something that someday someone might use .. those things tend to be forgotten, and break :P Did you have any thought on how that would work in the GUI? Personally, I am not against something like this; but this currently is just an idea that I guess needs to be worked out a bit further :) PS: if we do this for GS, we might as well do it for AIs too, but that is another story :) |
My idea of GUI is to have new text about GS in the right pane with game info, when server is selected. And new filter option for game script somewhere in the area where current server name filter is. But I am not really sure, whether to add the filter right away. Maybe I would start with adding just the UDP info and the simple new row, and when it is included, we could see how much are GS actually used across multiplayer games. And just after that add new filter. As for AI information, I would'n mix it with this patch. Are AI's even used in multiplayer games? While I can easily imagine people would search for specific GS, I am not so convinced if anyone would be interested in server with AI's. |
New version of gameinfo packet with extended detail about gamescript instead of new packet type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot better, tnx :) Now it also makes a lot more sense what you are doing :D
One thing that bothers me with adding gamesript info to game info packet, and the reason I haven't do that in the older fork, is that it breaks getting game info for any older openttd versions before this patch (if it would be included). For some reason I do not quite understand new stuff in info packet is added before the old. So when older version reads info packet from new hypothetical version, it will read GS info where it would currently expect NewGRF info, and thus fail. Solution would be to start adding any new info data to the end of the packet, so when older version reads new version packet, it reads what it knows and throw away the rest. But then it would not be consistent with current coding choice. What do we about that? I myself would prefer functionality of older versions before consistency of the code. |
Partial true what you mention. What happens is this: Older clients that request data from a newer servers, get "v5". They do not know how to read it, so skip reading it completely (they never hit any case in the switch). This is mostly always the case with protocol changes. It is backwards compatible, but not forwards compatible. That said, we are currently reworking some parts of the networking code, among which the GameInfo. In #9017 it is already bumping GameInfo to v5, and this can just be part of that bump. So v5 is already happening, so there is no reason to resolve it in another way :) |
Removing useles code Packet sends only version and GS name Change width of details right pane in lobby window (WID_NG_DETAILS) from 140 to 270px, so even longer strings are visible
Tnx! Most likely I cherry-pick this PR into #9017, or rebase it after that is merged. As that is already bumping game-info version, might as well make it part of the package. But that needs a bit more time, so stay tuned :) |
As promised, I integrated this in #9017. Will close this PR once that PR is merged :) |
Reopening the idea of UDP query of game script and probably showing it also in game GUI.
This patch focuses to extend fetching game info over UDP from multiplayer servers. Currently you can get basic game information and NewGRF details, but nothing about game scripts.
I think it would be useful to have possibility to found out, if server is running game scripts. When servers has a game scripts, it and could be shown in ingame lobby window and could be added also to web server listing .
Currently this patch solves it by adding new packet type, so there is extra UDP communication needed, but I think it would be better to rewrite it a little bit and incresase NETWORK_GAME_INFO_VERSION and add it to the end of the PACKET_SERVER_GAME_INFO packet in SerializeNetworkGameInfo() in game_info.cpp.