search

DaemonEngine / Daemon

The Dæmon game engine. With some bits of ioq3 and XreaL.
https://unvanquished.net
BSD 3-Clause "New" or "Revised" License
293 stars 60 forks source link

BIG_INFO_STRING is too short #1215

Open cu-kai opened 2 weeks ago

cu-kai commented 2 weeks ago

We are sending a lot of information in the info strings now, and 8192 is just not big enough for this as I found out, simply setting some g_disabledEquipment and g_disabledBuildables caused an overflow on Bunker, and players are either spammed by: "Warn: Unknown client game command ..." or kicked from the server with the "bcs exceeded BIG_INFO_STRING" message.

Can we increase this? (double it?) What are the implications of such a change?

sweet235 commented 2 weeks ago

How sad

slipher commented 2 weeks ago

I looked at the current serverinfo on Bunker and it's about 800 bytes. This is close to the "normal" MAX_INFO_STRING size of 1024, but nowhere near BIG_INFO_STRING. So probably the bcs commands are just broken and can't handle stuff more than 1024 correctly.

cu-kai commented 2 weeks ago

I looked at the current serverinfo on Bunker and it's about 800 bytes. This is close to the "normal" MAX_INFO_STRING size of 1024, but nowhere near BIG_INFO_STRING. So probably the bcs commands are just broken and can't handle stuff more than 1024 correctly.

Thanks for the clarification. So the real bug is not the one I am reporting? Feel free to re-title this as appropriate.

slipher commented 2 weeks ago

I was able to reproduce locally. The server sends a bcs0 and a bcs2 command, but somehow only the bcs2 command arrives to the client.

slipher commented 2 weeks ago

1218 fixes the bcs exceeded BIG_INFO_STRING bug, BUT it does not mean that you can safely have a serverinfo string up to 8192 characters. In other parts of the system the serverinfo string is limited to 1024 chars. It has more restrictions than a general config string.

With #1218 you can now safely have a serverinfo string which is close to 1024 characters but not exceeding it (this was probably your use case.)

Increasing the max serverinfo length would require a compatibility-breaking change. Anyway the idea of the serverinfo is to display only the things that would be most relevant to someone deciding which server to join.

DolceTriade commented 2 weeks ago

What is the long term fix? Are you saying we should create another mechanism for syncing arbitrary data between the server and client?

cu-kai commented 2 weeks ago

Increasing the max serverinfo length would require a compatibility-breaking change. Anyway the idea of the serverinfo is to display only the things that would be most relevant to someone deciding which server to join.

It seems to also be used to let the client decide which buildables, classes and equipment to show as locked in the menu.

What is the long term fix? Are you saying we should create another mechanism for syncing arbitrary data between the server and client?

This would probably be the best thing to do?

slipher commented 2 weeks ago

Other cvars that need to be sent to clients could be put in another info string besides the serverinfo one.