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

Score screen #1209

Merged
merged 13 commits into from
Jul 1, 2021
Merged

Conversation

GregoryGhost
Copy link
Contributor

@GregoryGhost GregoryGhost commented Jun 23, 2021

Part of #1120
Part of #1121

Changes

  • ...

Checklist

@codecov
Copy link

codecov bot commented Jun 23, 2021

Codecov Report

Merging #1209 (c957cac) into feature-1121-score-screen (1fcec6e) will increase coverage by 0.15%.
The diff coverage is n/a.

❗ Current head c957cac differs from pull request most recent head 3ba5fec. Consider uploading reports for the commit 3ba5fec to get more accurate results
Impacted file tree graph

@@                      Coverage Diff                      @@
##           feature-1121-score-screen    #1209      +/-   ##
=============================================================
+ Coverage                      63.20%   63.35%   +0.15%     
=============================================================
  Files                            345      347       +2     
  Lines                           7866     7901      +35     
=============================================================
+ Hits                            4972     5006      +34     
- Misses                          2894     2895       +1     
Flag Coverage Δ
Zilon.Core.FunctionalTests 39.89% <ø> (-0.12%) ⬇️
Zilon.Core.UnitTests 48.64% <ø> (+1.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
Zilon.Core/Zilon.Core/PerkHelper.cs 80.30% <0.00%> (-13.04%) ⬇️
...ilon.Core/Zilon.Core/Tactics/HumanSectorFowData.cs 79.16% <0.00%> (-9.21%) ⬇️
...llularAutomatonStyle/InteriorObjectRandomSource.cs 81.25% <0.00%> (-4.72%) ⬇️
...ore/Zilon.Core/World/ResourceMaterializationMap.cs 96.82% <0.00%> (-1.66%) ⬇️
Zilon.Core/Zilon.Core/World/BiomeSchemeRoller.cs 100.00% <0.00%> (ø)
...on.Core/Zilon.Bot.Players/Logics/ExitLogicState.cs 0.00% <0.00%> (ø)
...on.Core/Zilon.Core/Tactics/MonsterSectorFowData.cs 0.00% <0.00%> (ø)
...on.Core/Zilon.Core/World/GlobeTransitionHandler.cs 73.07% <0.00%> (ø)
...Core/Zilon.Core/PersonModules/IPersonExtensions.cs 72.72% <0.00%> (ø)
...Core/MapGenerators/SwitchMapFactorySelectorBase.cs 0.00% <0.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fcec6e...3ba5fec. Read the comment docs.

@coveralls
Copy link

coveralls commented Jun 23, 2021

Pull Request Test Coverage Report for Build 964811730

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 59.811%

Totals Coverage Status
Change from base Build 962717650: 0.0%
Covered Lines: 5006
Relevant Lines: 7901

💛 - Coveralls

@kreghek kreghek changed the base branch from master to feature-1121-score-screen June 23, 2021 16:04
_spriteBatch = spriteBatch;

var buttonTexture = game.Content.Load<Texture2D>("Sprites/ui/button");
var font = Game.Content.Load<SpriteFont>("Fonts/Main");
Copy link
Owner

Choose a reason for hiding this comment

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

You should get main font and button texture from IUiContentStorage to prevent multiple loading of same resources.

title: UiResources.NextScreenButtonTitle,
texture: buttonTexture,
font: font,
rect: new Rectangle(x: 550, y: 150, width: 100, height: 20));
Copy link
Owner

Choose a reason for hiding this comment

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

Do not use magic numbers. Instead use named constants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What are magic numbers? It's just a position of the button and other parameters.

Copy link
Owner

Choose a reason for hiding this comment

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

const int BUTTON_WIDTH = 100;
const int BUTTON_HEIGHT = 20;
const int BUTTON_STACK_X = 150;
const int BUTTON_STACK_Y = 150;

_goToNextScreen = new TextButton(
                title: UiResources.NextScreenButtonTitle,
                texture: buttonTexture,
                font: font,
                rect: new Rectangle(
                     x: BUTTON_STACK_X  + BUTTON_WIDTH * 2 ,
                     y: BUTTON_STACK_Y ,
                     width: BUTTON_WIDTH,
                     height: BUTTON_HEIGHT));

It's better to maintain and understand that just x: 550, y: 150, width: 100, height: 20

Comment on lines 103 to 109
var state = Keyboard.GetState();

// If they hit esc, exit
if (state.IsKeyDown(Keys.Escape))
{
Game.Exit();
}
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like copy-pasta. We should exit game the Keys.Escape.


_spriteBatch.DrawString(
spriteFont: font,
text: "Score menu",
Copy link
Owner

Choose a reason for hiding this comment

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

Localize all output strings via UiResources. The easiest way to manage resources is ResX extension described in README.md

@kreghek kreghek self-assigned this Jun 23, 2021
@kreghek kreghek added the client Works on client label Jun 23, 2021
title: UiResources.NextScreenButtonTitle,
texture: buttonTexture,
font: font,
rect: new Rectangle(x: 550, y: 150, width: 100, height: 20));
Copy link
Owner

Choose a reason for hiding this comment

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

const int BUTTON_WIDTH = 100;
const int BUTTON_HEIGHT = 20;
const int BUTTON_STACK_X = 150;
const int BUTTON_STACK_Y = 150;

_goToNextScreen = new TextButton(
                title: UiResources.NextScreenButtonTitle,
                texture: buttonTexture,
                font: font,
                rect: new Rectangle(
                     x: BUTTON_STACK_X  + BUTTON_WIDTH * 2 ,
                     y: BUTTON_STACK_Y ,
                     width: BUTTON_WIDTH,
                     height: BUTTON_HEIGHT));

It's better to maintain and understand that just x: 550, y: 150, width: 100, height: 20

@@ -54,7 +54,7 @@ private void ProcessIfHealth(SurvivalStat stat, int valueDiff)
return;
}

var hp = stat.Value;
var hp = 0; //stat.Value;
Copy link
Owner

Choose a reason for hiding this comment

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

??
I thing this is test changes. This is not for master branch.
it's better to test via cheat input component. There is no way to manipulate person attributes. But you can add wanted cheat easely.

…Roguelike into score-screen

� Conflicts:
�	Zilon.Core/CDT.LAST.MonoGameClient/Screens/LeaderBoardScreen.cs
�	Zilon.Core/CDT.LAST.MonoGameClient/Screens/ScoresScreen.cs
@kreghek kreghek merged commit 534e2f7 into kreghek:feature-1121-score-screen Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Works on client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants