Patches #1936

xonotic-data.pk3dir:Mario/gametype_votes: a L merge request: Adds a gametype vote screen before the map vote screen for easy gametype...

Added by git-manager 5 months ago. Updated 2 months ago.

Status:FeedbackStart date:03/20/2014
Priority:NormalDue date:
Assignee:divVerent% Done:

0%

Category:-
Target version:Xonotic - 0.8

Description

Adds a gametype vote screen before the map vote screen for easy gametype switching.
Disabled by default, the options and timers can be customized, with support for an almost infinite number of maps/modes.

Repository: xonotic/xonotic-data.pk3dir.git
Commit: 84d3168e45d3923201fe690255afdb656c708615
Branch: Mario/gametype_votes

Merge commands:

cd data/xonotic-data.pk3dir
git checkout master
git reset --hard origin/master
git pull && git diff '84d3168e45d3923201fe690255afdb656c708615'..'origin/Mario/gametype_votes'

# please check that the diff you just saw did not contain anything complex that
# needs a new merge request, and review these changes

git merge --edit --log --no-ff 'origin/Mario/gametype_votes'

# please make sure this merge worked, and if not, fix merge conflicts and git
# commit BEFORE the next command
#
# also, THIS is the point to do final pre-merge testing
#
# use git reset --hard origin/master to bail out

git push && git push --delete origin 'Mario/gametype_votes'

Diffstat:

 defaultXonotic.cfg        |    6 +
 gamemodes.cfg             |   22 ++
 qcsrc/client/mapvoting.qc |  358 +++++++++++++++++++----
 qcsrc/common/constants.qh |    7 +-
 qcsrc/common/mapinfo.qc   |   16 +-
 qcsrc/common/mapinfo.qh   |   37 ++-
 qcsrc/server/autocvars.qh |    5 +
 qcsrc/server/g_world.qc   |  543 ++--------------------------------
 qcsrc/server/mapvoting.qc |  728 +++++++++++++++++++++++++++++++++++++++++++++
 qcsrc/server/mapvoting.qh |   39 +++
 qcsrc/server/progs.src    |    4 +
 11 files changed, 1162 insertions(+), 603 deletions(-)

Revision log:

commit 84d3168e45d3923201fe690255afdb656c708615
Author: Mario <mario.mario@y7mail.com>
Commit: Mario <mario.mario@y7mail.com>

    Gametype vote screen by Melanosuchus

User agreed to the license.

Diff:

*** DIFF TOO LARGE, PLEASE CHECK YOURSELF ***

History

#1 Updated by Mario 5 months ago

  • Assignee set to terencehill

Assigning to terencehill for initial review.

#2 Updated by Mario 3 months ago

  • Target version set to 0.8

#3 Updated by terencehill 3 months ago

These strings got removed but maybe they are useful to highlight any bug that will arise. DivVerent should know better wether they can go or not.
- mv_maps[i] = strzone("if-you-see-this-the-code-is-broken");
- mv_pk3[i] = strzone("if-you-see-this-the-code-is-broken");
- mv_pics[i] = strzone("if-you-see-this-the-code-is-broken");

draw_UseSkinFor(map) instead of:
+ //map = strzone(strcat("gfx/menu/default/gametype_", map));
+ map = strzone(sprintf("gfx/menu/%s/gametype_%s", autocvar_menu_skin, map));
There's no support for fallback images in draw_UseSkinFor either, probably because it uses menu images... don't care about it for now.

Arrow keys shouldn't trigger any vote, they should just change the selection, then K_KP_ENTER, K_ENTER or K_SPACE to actually change the vote. Better if key selection starts from the current mouse selection, e.g. if the mouse selected item is #2 then right arrow should select #3.

I've created a new branch terencehill/gametype_votes where the patch differs from Mario/gametype_votes for the fact that I moved code from the new file qcsrc/server/mapvoting.qc back to qcsrc/server/g_world.qc. This way the patch can be reviewed more sanely. Besides removing some trailing whitespace I didn't make any other change. The map voting code can be moved to qcsrc/server/mapvoting.qc later.

#4 Updated by Melanosuchus 3 months ago

Regarding the issue about if-you-see-this-the-code-is-broken, I made some changes on how the map options are sent to the clients.
Currently if the bug which caused "the-code-is-broken" occurs, the maps will be shown as usual but they won't be votable.
In my opinion this looks better and it's still obvious that something is not working properly. Unfortunately I have not been able to locate the actual source of the issue as it occurs quite rarely.
The change is caused by the fact that this branch makes allows more than 9 votable options in the voting screen.

#5 Updated by Mario 3 months ago

The skin and selection issues have been fixed.

#6 Updated by divVerent 2 months ago

Found this really bad thing on first glance:

mapinfo.qc:MapInfo_Get_ByName:
if(!(MapInfo_Map_supportedGametypes & pGametypeToSet)) {
- error("Can't select the requested game type. This should never happen as the caller should prevent it!\n");
+ //error("Can't select the requested game type. This should never happen as the caller should prevent it!\n");
+ return 0;
//_MapInfo_Map_ApplyGametypeEx("", pGametypeToSet, MAPINFO_TYPE_DEATHMATCH);
//return;
}

This is very bad and hides critical errors. Please undo this.

Also, your code randomly changes cvars like fraglimit/timelimit/gametype-specific-stuff/settemps when nextmap is set. This can be abused for quite some evil.

Instead, fix GameTypeVote_AvailabilityStatus (I presume this is the only caller causing this problem): pass 0 as pGametypeToSet, and check the MapInfo_Map_supportedGametypes bitmask after the call. The gametype in this function call REALLY should only be set in the final call to apply cvar changes!

Example:

if ( autocvar_nextmap != "" )
{
if ( !MapInfo_Get_ByName(autocvar_nextmap, FALSE, 0) )
return GTV_FORBIDDEN;
if (!(MapInfo_Map_supportedGametypes & type))
return GTV_FORBIDDEN;
}

Remember that the third argument of MapInfo_Get_ByName is called pGametypeToSet, and it will actually SET that given gametype! You really don't want to do that here.

#7 Updated by divVerent 2 months ago

As for the general code design: Looks fine.

I'll review the changes to outside systems, and interaction with Mapinfo, later. Mapinfo-internal stuff is probably fine if it doesn't crash all the time - and as its design seems fine, if it breaks, we can fix it.

Other things, not necessarily to be done in this branch:
- Merge/link the votable list with the list of gametypes to show in the menu. If we hide a gametype from the menu, we want it to be automatically hidden from vote screens too (unless explicitly enabled by the admin).
- server.cfg changes for this (in another repo, I know).
- Gametype descriptions are a great idea. Can we have them somewhere in the menu too? Ingame, too (e.g. on the info key)?

#8 Updated by Mario 2 months ago

  • Status changed from New to Feedback
  • Assignee changed from terencehill to divVerent

The 2 major issues listed have been fixed.

Also available in: Atom PDF