Skip to content

Moved Remove operation to start of Table.Apply #2967

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

Merged
merged 2 commits into from
Jul 24, 2025

Conversation

rekhoff
Copy link
Contributor

@rekhoff rekhoff commented Jul 21, 2025

Description of Changes

Intends to resolve issue 343 of the C# SDK.

In issue 343, the example project uses fails due the PlayerFruitState table being defined with an Identity value is marked as unique rather than primary_key as is typically the case. Without defining a primary key, data from all rows is used as the primary key.

In the sample project, when the PlayerFruitState table was being updated, the Identity value would stay the same, and the other values would change. This should still be a supported operation, and would look like a removal of the Identity row with the old value and an addition of the Identity row with the new value, rather than a single update operation.
Note: If the Identity was marked as primary_key, this would be an update operation, because only the Identity value would be used for the key, rather than all rows.

Because of the existing order of operations in a Table.cs's Apply method, the new row being added to the PlayerFruitState table prior to the old row being removed. Due to the Identity value having the unique constraint, this caused the initial add to throw an error during the add operation.

This change simply moves the Remove operation before Add, which is best-practice for these types of operations.

API and ABI breaking changes

Not API breaking.

Expected complexity level and risk

1

Testing

  • Tested against "FruitMerge3D" sample project

@rekhoff rekhoff requested review from jdetter and gefjon July 21, 2025 20:40
Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

I gave this a try using my fruit merge 3D example and it works now, thanks Ryan! 👍

@rekhoff rekhoff added this pull request to the merge queue Jul 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 23, 2025
@rekhoff rekhoff added this pull request to the merge queue Jul 24, 2025
Merged via the queue into master with commit 62a1378 Jul 24, 2025
25 checks passed
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.

Incremental updates throw error - Item with the same key has already been added
2 participants