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

Add Dice game #2

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Add Dice game #2

wants to merge 19 commits into from

Conversation

alexyakunin
Copy link
Owner

No description provided.

Copy link
Owner Author

@alexyakunin alexyakunin left a comment

Choose a reason for hiding this comment

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

Wrote some comments.

public static DiceBoard Empty(int size) => EmptyCache.GetOrAdd(size, size1 => new DiceBoard(size1));

public int Size { get; }
public Dictionary<int, string[]> Cells { get; }
Copy link
Owner Author

Choose a reason for hiding this comment

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

Most of Game* types are immutable, this dictionary is mutable.

get {
var cellIndex = GetCellIndex(r, c);
if (cellIndex < 0 || cellIndex >= Cells.Count)
return new string[] {"lightblue", "lightblue", "lightblue", "lightblue"};
Copy link
Owner Author

Choose a reason for hiding this comment

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

IMO it's a bad idea to use string as underlying player color or something like this. Maybe an enum?


public DiceBoard(int size)
{
var defaultValue = "lightblue";
Copy link
Owner Author

Choose a reason for hiding this comment

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

Not sure what "lightblue" means :) Color? Empty cell?

@@ -17,7 +17,7 @@ public class HostSettings

// DBs
public string UsePostgreSql { get; set; } =
"Server=localhost;Database=board_games_dev;Port=5432;User Id=postgres;Password=Fusion.0.to.1";
"Server=localhost;Database=board_games_dev;Port=5432;User Id=postgres;Password=pg051825";
Copy link
Owner Author

Choose a reason for hiding this comment

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

IMO you shouldn't change the default password that matches the password from docker-compose.yml

Copy link
Owner Author

Choose a reason for hiding this comment

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

Maybe just use the original docker-compose.yml?

@@ -75,6 +75,8 @@ public void ConfigureServices(IServiceCollection services)
HostSettings = tmpServices.GetRequiredService<HostSettings>();

// DbContext & related services

// HostSettings.UseSqlite = true;
Copy link
Owner Author

Choose a reason for hiding this comment

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

This commented out piece should be removed.

@attribute [MatchFor(typeof(DiceEngine), ComponentScopes.GameRules)]
@inherits GameRulesBase

<p><b>Rules</b>: Get to the finish line first.</p>
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok, I'll write a bit more details here :)

@@ -20,6 +20,7 @@ public override void Use()
{
// Game engines
Services.TryAddEnumerable(ServiceDescriptor.Singleton<IGameEngine, GomokuEngine>());
Services.TryAddEnumerable(ServiceDescriptor.Singleton<IGameEngine, DiceEngine>());
Copy link
Owner Author

Choose a reason for hiding this comment

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

FYI this changed in the latest game verison.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Check out the attributes on GomokuEngine & copy them to get this working.

public static DiceBoard Empty(int size) => EmptyCache.GetOrAdd(size, size1 => new DiceBoard(size1));

public int Size { get; }
public ImmutableDictionary<int, Cell> Cells { get; }
Copy link
Owner Author

Choose a reason for hiding this comment

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

You don't need a dict if you use 100% index range from 0 to N :) You need an array :)


public enum Colors
{
Blue,
Copy link
Owner Author

Choose a reason for hiding this comment

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

I would rather use CellStates vs Colors. Color is meaningless, but cell state - e.g. "BoostCell" - is meaninful.

public int Size { get; }
public ImmutableDictionary<int, Cell> Cells { get; }

public struct Cell
Copy link
Owner Author

Choose a reason for hiding this comment

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

This thing definitely requires some serialization-related optimizations. Or maybe DiceBoard.

public struct Cell
{
public string Background;
public string[] Colors;
Copy link
Owner Author

Choose a reason for hiding this comment

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

Why cell has multiple colors?

Copy link
Collaborator

Choose a reason for hiding this comment

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

4 colors for all players (blue, green, red, yellow)

{
public string Background;
public string[] Colors;
public double[] Opacities;
Copy link
Owner Author

Choose a reason for hiding this comment

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

Same for opacities?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Each cell has 1 background color and 4 "color + opacity" units.

Size = size;
var builder = ImmutableDictionary.CreateBuilder<int, Cell>();
for (int i = 0; i < size * size; i++) {
if (i == 10 || i == 27 || i == 44) { // ForwardStep cells
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'd make this random.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Random can make infinite loop.

var playerScore = oldPlayerScore += move.Value;
state.Steps[state.PlayerIndex] += 1;
var playerSteps = state.Steps[state.PlayerIndex];
if (playerScore == 10 || playerScore == 27 || playerScore == 44) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is kinda weird - i.e. I'd use board to check exactly this thing.

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

Successfully merging this pull request may close these issues.

2 participants