xyaneon / xyaneon.games.cards Goto Github PK

For a while now, semantic-release has not been working for this repository, mainly due to apparent token issues despite attempts to fix its setup. Either a working solution should be found or it should be removed (and preferably find a replacement).

Have the StandardPlayingCard class implement the IEquatable<StandardPlayingCard> interface.

I tried to check this on my local machine, but the only output I am getting from the test command (same as on main) is:

  Determining projects to restore...
  All projects are up-to-date for restore.

This is even with trying to deliberately introduce a test failure or skip the restore step, which produces no output at all. This is after reinstalling .NET 6 on my Ubuntu 22.04 laptop with "Mixed state scenario 1" mentioned on dotnet/core#7699 . I do not see the test results in the pipeline either where a clean installation is supposed to be automatically set up, so this might be some .NET bug recently introduced.

At any rate, I would not merge this until the testing issues can be resolved.

Originally posted by @Xyaneon in #41 (comment)

The NuGet package should have an icon to display.

Revert commit 11a3c03, as Bionic is now supported by Travis CI as the newest LTS release (even though at this time it is still not yet the default).

Coveralls is reporting that the DefaultShuffleAlgorithm class has no unit test coverage as of the first build on main with it enabled.

.NET 5 has now reached end-of-life. The project should be upgraded to .NET 6+.

It might be helpful to create a sample project or two showing the basics of using this library. Could be something as simple as a CLI implementation of Go Fish or War.

Something using a custom deck would also work and better illustrate how to effectively subclass the Card class, assuming there is something simple I could implement without having to worry about copyright issues.

The README.md file should have sections added with examples showing how to use this library.

The Xyaneon.Games.Cards.Test project contains a lot of comments which are not needed, and could be removed to help shorten the code a bit.

The following can be removed:

• XML doc comments: The test names should give a clear enough description of what the tests are actually for, and I doubt we would need to generate documentation from test methods.
• Arrange/Act/Assert comments: The tests should still follow the AAA pattern for readability, but the additional comments calling out each section are a bit redundant and repetitive.

The following is reported by Travis CI at the bottom of the build log:

Skipping a deployment with the script provider because a custom condition was not met

[Job reference]

Building the NuGet package for this project outputs a warning like this:

warning NU5048: The 'PackageIconUrl'/'iconUrl' element is deprecated. Consider using the 'PackageIcon'/'icon' 
element instead. Learn more at https://aka.ms/deprecateIconUrl

The provided link suggests also defining PackageIcon and having the icon bundled into the package, with PackageIconUrl left in for backwards compatibility.

This package already has continuous integration (CI) set up using Travis CI. See if this can be modified to also do continuous deployment (CD).

The *.nupkg and *.snupkg files were part of the GitHub Release for versions 1.0.0 and previous as attached assets, but they are missing for the 1.1.0 release. They were at least uploaded to NuGet by the CD pipeline, however.

The GitHub Release will need these packages to be manually added, and some troubleshooting will be needed to figure out why these were skipped this time around.

As I was working with DrawPile, I noticed that when I went to place
a card on the bottom it was placed on the top again.

After looking into it, the pile's cards were being put in the same order
as before the card was drawn and inserted.

Relevant code

I know it was originally a delegate before I merged #1, but now I cannot remember why I did that in the first place. What the library is now currently using more closely resembles the strategy pattern, but I am not sure if it is actually the best design.

Note that changing this would be a breaking change requiring a new major version bump.

After merging to main, if Semantic Release determines from the commits that there is no new package version to release, the subsequent build steps for rebuilding the package with the new version it would have provided show as failed.

Just as there is an IDrawPile<TCard>.Insert(int, TCard) method, perhaps there should also be a method for pulling a card out of the deck at a given index. Imagine card games where someone might want to pull a random card out from the middle somewhere.

See available symbols here, which can be made available via a utility class providing constants and/or static methods: https://en.wikipedia.org/wiki/Playing_cards_in_Unicode

Coveralls currently shows that DrawPile.cs only has 48.62% coverage since it has been first added. This should be increased.

Hello!

I get that decks are supposed to contain the concept of a card, (like a traditional playing card) but does it need to be an abstract class? The reason I ask this is because it can cause a few issues if someone has already defined their own objects that need to inherit from some other base class (Such as a GameObject), then they can't use this library very easily.

There's two thoughts I have for fixing this (And to keep from breaking changes, even). The first is to add an interface such as ICard, and have your abstract class inherit from ICard, and change the shuffling algorithm to use where TCard : ICard, then a person could add the ICard interface to any object they want easily.

Or Option 2, why specify that it needs to inherit from anything at all? A deck doesn't have to be cards. (Anything could be shuffled, really, like songs). It seems that the underlying data structure it accepts is just IEnumerable, just let people use any objects with this interface?

I appreciate your thoughts on the matter.

This should include the following:

• Should be done using GitHub Actions, so the Travis CI files can be finally removed per #21
• Should create a new GitHub Release following Semantic Versioning
• Should publish a new NuGet package to nuget.org (and optionally to GitHub Packages)

Due to changes in Travis CI pricing for OSS repositories, this repository should switch to GitHub Actions for PR checks and the release pipeline.