Skip to content

Conversation

Absolutionism
Copy link
Contributor

@Absolutionism Absolutionism commented Sep 6, 2024

Description

This PR aims to restrict the usage of a file to only one database. Precautionary measure

  • Now also fixes the databaseName field and adds a databaseType field

Target Minecraft Versions: any
Requirements: none
Related Issues: #6858

Fix, the databaseName being set as the type instead of the name
Added, databaseType field
@Absolutionism
Copy link
Contributor Author

Please note, to optimally fix the databaseName and the databaseType field added through this PR, by simply setting within the constructor, will be a "breaking change" - LimeGlass

Copy link
Contributor

@TheLimeGlass TheLimeGlass left a comment

Choose a reason for hiding this comment

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

Looking good.

Dev note: The breaking change is because the field behavior is being changed from type to configuration name, thus the field should be private to avoid anyone extending VariableStorage to be using old behavior.
The current VariableStorage API is very recent, and this change is minimal.

@TheLimeGlass TheLimeGlass added enhancement Feature request, an issue about something that could be improved, or a PR improving something. breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) labels Sep 7, 2024
Added, Javadoc note for getUserConfigurationName()
Changed, database(Name|Type) fields to private
Changed, all usages of 'databaseName' to 'getUserConfigurationName()'
Changed, 'registeredFiles' to Set
Changed, 'name' var
@Absolutionism
Copy link
Contributor Author

Question, should we also restrict the usage of same database names?

Forgot to change the initial constructor call back to original
@Efnilite Efnilite added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Nov 8, 2024
Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

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

Looks fine.

@sovdeeth sovdeeth merged commit e40ef23 into SkriptLang:dev/feature Dec 15, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants