Skip to content
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

Make scoring only possible with a higher amount of players on server #144

Merged
merged 3 commits into from
Mar 27, 2019
Merged

Make scoring only possible with a higher amount of players on server #144

merged 3 commits into from
Mar 27, 2019

Conversation

ResamVi
Copy link
Collaborator

@ResamVi ResamVi commented Mar 24, 2019

Would solve #135

@ResamVi
Copy link
Collaborator Author

ResamVi commented Mar 24, 2019

A better solution would be to add a column PlayerCount to s_infc_RoundScore and when a winner of the of-the-day-challenge is determined WHERE TableScore.PlayerCount > 8

But as said. I can test if the code compiles. I can't test if it works because I still havent figured out how to init the databases

Need to tell me what solution should be implemnted and somebody else got to test it then, though.

@ResamVi
Copy link
Collaborator Author

ResamVi commented Mar 25, 2019

@teoman002 maybe you decide which solution because you tagged me :D or @bretonium knows if changing the database schema causes trouble

@teoman002
Copy link
Collaborator

solution 1 seems simple, so I suggest to do that. We can improve the code later™.

@teoman002 teoman002 added enhancement New feature or request ready to merge This pr is ready to be merged labels Mar 25, 2019
teoman002
teoman002 previously approved these changes Mar 26, 2019
src/engine/server/server.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@bretonium bretonium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, it does not compile

@bretonium
Copy link
Collaborator

changing the database schema causes trouble

By itself it doesn't, but we do not have the necessary infrastructure to automate schema migration, and asking administrators to do the migration manually, via .sql file, is a pain. We could add a check that the migration was applied and exit if it wasn't, but it is too much hassle for a simple issue.

@teoman002 teoman002 self-requested a review March 26, 2019 12:41
@teoman002 teoman002 dismissed their stale review March 26, 2019 12:42

does not compile

@ResamVi ResamVi requested a review from bretonium March 26, 2019 15:00
Copy link
Collaborator

@bretonium bretonium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be good

@teoman002 teoman002 removed their request for review March 26, 2019 20:56
@bretonium bretonium merged commit c3950d4 into yavl:master Mar 27, 2019
@teoman002 teoman002 removed the ready to merge This pr is ready to be merged label Mar 31, 2019
@@ -4218,7 +4218,10 @@ class CSqlJob_Server_SendPlayerStatistics : public CSqlJob
void UpdateScore(CSqlServer* pSqlServer, int ScoreType, int Score, const char* pScoreName)
{
char aBuf[512];


if (m_pServer->GetActivePlayerCount() > 8)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

> 8
LOL



if (m_pServer->GetActivePlayerCount() > 8)
return
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what this line without ; in the end does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants