Skip to content

fix(database): prevent dupe, and add new placeholders #47

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.

Sign up for GitHub

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

Closed
Valjean42 wants to merge 4 commits into from
Closed

fix(database): prevent dupe, and add new placeholders #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a84ffb7
API Update, a few DB-related fixes (I'm not entirely sure about them,…
Valjean42 Jul 3, 2025
edad0ba
Removed two Delete calls that danil said are unneeded
Valjean42 Jul 4, 2025
e0e9006
Also register the permission used for admin actions
Valjean42 Jul 7, 2025
db1733a
Extra fix!
Valjean42 Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ subprojects {
apply plugin: 'maven-publish'

group = 'com.envyful.gts'
version = '5.1.0'
version = '5.2.0'
Copy link
Member

@danorris709 danorris709 Jul 15, 2025

Choose a reason for hiding this comment

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

Not important for now but in the future you shouldn't include these changes in a PR

Copy link
Author

@Valjean42 Valjean42 Jul 15, 2025

Choose a reason for hiding this comment

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

kk :3


ext {
forgeVersion = "none"
Expand Down
6 changes: 3 additions & 3 deletions neo21/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ sourceSets.main.resources { srcDir 'src/generated/resources' }
repositories {}

dependencies {
implementation "pixelmon:Pixelmon-1.21.1-9.3.2-universal:9.3.2"
interfaceInjectionData("pixelmon:Pixelmon-1.21.1-9.3.2-universal-interfaceinjection:9.3.2@json")
implementation "pixelmon:Pixelmon-1.21.1-9.3.5-universal:9.3.5"
interfaceInjectionData("pixelmon:Pixelmon-1.21.1-9.3.5-universal-interfaceinjection:9.3.5@json")

implementation "com.envyful.papi:api:2.1.0"

shadow group: 'org.spongepowered', name: 'configurate-yaml', version: '4.2.0-COMMENTS'

var apiVersion = "8.0.9-t1"
var apiVersion = "8.1.8"
shadow (group: 'com.envyful.api', name: 'commons', version: apiVersion)
shadow (group: 'com.envyful.api', name: 'neo21', version: apiVersion) {
transitive = false
Expand Down
2 changes: 2 additions & 0 deletions neo21/src/main/java/com/envyful/gts/forge/EnvyGTSForge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.envyful.api.neoforge.platform.ForgePlatformHandler;
import com.envyful.api.neoforge.player.ForgeEnvyPlayer;
import com.envyful.api.neoforge.player.ForgePlayerManager;
import com.envyful.api.neoforge.player.util.UtilPlayer;
import com.envyful.api.platform.PlatformProxy;
import com.envyful.api.player.Attribute;
import com.envyful.api.sqlite.config.SQLiteDatabaseDetailsConfig;
Expand Down Expand Up @@ -127,6 +128,7 @@ public void loadConfig() {

@SubscribeEvent
public void onServerStarting(RegisterCommandsEvent event) {
UtilPlayer.registerPermission("envygts.admin");
this.commandFactory.registerCommand(event.getDispatcher(), this.commandFactory.parseCommand(new GTSCommand()));
}

Expand Down
1 change: 1 addition & 0 deletions neo21/src/main/java/com/envyful/gts/forge/impl/trade/ForgeTrade.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.envyful.gts.forge.impl.trade;

import com.envyful.api.concurrency.UtilConcurrency;
import com.envyful.api.neoforge.chat.UtilChatColour;
import com.envyful.api.neoforge.player.ForgeEnvyPlayer;
import com.envyful.api.player.EnvyPlayer;
Expand Down
14 changes: 7 additions & 7 deletions neo21/src/main/java/com/envyful/gts/forge/impl/trade/type/ItemTrade.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,20 @@ public CompletableFuture<Void> collect(EnvyPlayer<?> player, Consumer<EnvyPlayer
if (!parent.getInventory().add(copy)) {
player.message(UtilChatColour.colour(EnvyGTSForge.getLocale().getMessages().getInventoryFull()));

if (returnGui == null) {
parent.closeContainer();
} else {
returnGui.accept(player);
}
UtilConcurrency.runAsync(this::delete);
Copy link
Member

@danorris709 danorris709 Jul 15, 2025

Choose a reason for hiding this comment

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

Delete call should not be here. The stack was not fully returned to the player's inventory

Copy link
Author

@Valjean42 Valjean42 Jul 15, 2025

Choose a reason for hiding this comment

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

It saves immediately afterwards. This deletion is for when the item partially got added, but some still couldn't be, which causes the item's count to be different, so we have to reflect that in the DB

Copy link
Member

@danorris709 danorris709 Jul 16, 2025

Choose a reason for hiding this comment

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

Yeah so we shouldn't be deleting it, we should just be updating the value in the database which is what the save call should be doing

Copy link
Author

@Valjean42 Valjean42 Jul 16, 2025

Choose a reason for hiding this comment

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

ok, this might just be me being SQL stupid, but does INSERT INTO update existing values? I thought it just creates a new one.

Copy link
Member

@danorris709 danorris709 Jul 16, 2025

Choose a reason for hiding this comment

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

Yes you are correct it is not updating. Perhaps this is the root cause of the issue? I have been assuming the save call was an INSERT INTO .... ON DUPLICATE KEY UPDATE etc etc

Copy link
Author

@Valjean42 Valjean42 Jul 16, 2025

Choose a reason for hiding this comment

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

possibly the cause of the issue. hmm.


this.item.setCount(copy.getCount());

var attribute = player.getAttributeNow(GTSAttribute.class);
attribute.getOwnedTrades().add(this);
this.save();

if (returnGui == null) {
parent.closeContainer();
} else {
returnGui.accept(player);
}

return CompletableFuture.completedFuture(null);
}

Expand All @@ -95,8 +97,6 @@ public CompletableFuture<Void> collect(EnvyPlayer<?> player, Consumer<EnvyPlayer
returnGui.accept(player);
}

UtilConcurrency.runAsync(this::save);

return UtilConcurrency.runAsync(this::delete);
}

Expand Down
3 changes: 2 additions & 1 deletion neo21/src/main/java/com/envyful/gts/forge/impl/trade/type/PokemonTrade.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,9 @@ public void displayClaimable(int pos, Consumer<EnvyPlayer<?>> returnGui, Pane pa
.asyncClick(false)
.singleClick()
.clickHandler((envyPlayer, clickType) -> {
GTSAttribute attribute = ((ForgeEnvyPlayer) envyPlayer).getAttributeNow(GTSAttribute.class);
GTSAttribute attribute = envyPlayer.getAttributeNow(GTSAttribute.class);
attribute.getOwnedTrades().remove(this);
UtilConcurrency.runAsync(this::delete);
Copy link
Member

@danorris709 danorris709 Jul 15, 2025

Choose a reason for hiding this comment

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

This shouldn't be here. We should only delete the trade once we are certain that the entire stack has been added to their inventory

Copy link
Author

@Valjean42 Valjean42 Jul 15, 2025

Choose a reason for hiding this comment

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

I think if the trade is removed from getOwnedTrades the db should reflect the same changes no?

Copy link
Author

@Valjean42 Valjean42 Jul 15, 2025

Choose a reason for hiding this comment

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

Also this is the pokemon trade, there is no stack.

Copy link
Member

@danorris709 danorris709 Jul 16, 2025

Choose a reason for hiding this comment

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

Yeah I didn't look at the class name there. However, I think this is still applicable as what if their PC is full?

Copy link
Author

@Valjean42 Valjean42 Jul 16, 2025

Choose a reason for hiding this comment

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

I think the current code actually doesn't handle that at all?

this.collect(envyPlayer, returnGui);
})
.build());
Expand Down
3 changes: 2 additions & 1 deletion neo21/src/main/java/com/envyful/gts/forge/ui/EditDurationUI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static void openUI(ForgeEnvyPlayer player, int page, int position, boolea
.replace("%max_duration%", UtilTimeFormat.getFormattedDuration(
TimeUnit.SECONDS.toMillis(EnvyGTSForge.getConfig().getMinTradeDuration())
))
.replace("%pokemon%", pokemon.getDisplayName().getString())))
.replace("%pokemonItem%", pokemon.getDisplayName().getString())))
Copy link
Member

@danorris709 danorris709 Jul 15, 2025

Choose a reason for hiding this comment

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

Why have you changed this?

Copy link
Author

@Valjean42 Valjean42 Jul 15, 2025

Choose a reason for hiding this comment

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

Both types of DurationUIs use the same locale string, so I changed these two (this one and the question below) to be the same replacement string. Maybe I should change it to be something like "tradableName" to be a little more generic...

Copy link
Member

@danorris709 danorris709 Jul 16, 2025

Choose a reason for hiding this comment

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

Ah I see.

Yeah perhaps an underscore inbetween the words too to match the format of the other placeholders?

.defaultText(TimeUnit.SECONDS.toMinutes(EnvyGTSForge.getConfig().getMinTradeDuration()) + "m")
.maxInputLength(10)
.closeOnEscape()
Expand Down Expand Up @@ -96,6 +96,7 @@ public static void openUI(ForgeEnvyPlayer player, int page, int position, boolea
attribute.setCurrentMinPrice(0);
attribute.setCurrentPrice(0);
attribute.setSelectedSlot(-1);
PlatformProxy.runSync(player.getParent()::closeContainer);
Copy link
Member

@danorris709 danorris709 Jul 15, 2025

Choose a reason for hiding this comment

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

Why did you add this here? Clicking the button should already close the UI

Copy link
Author

@Valjean42 Valjean42 Jul 15, 2025

Choose a reason for hiding this comment

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

Oh, that was me trying to prevent the error that pops up with dialogues (which didn't work), I guess I didn't notice and committed it too.

});
})
.build()
Expand Down
5 changes: 3 additions & 2 deletions neo21/src/main/java/com/envyful/gts/forge/ui/EditItemDurationUI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.envyful.api.neoforge.concurrency.UtilForgeConcurrency;
import com.envyful.api.neoforge.player.ForgeEnvyPlayer;
import com.envyful.api.neoforge.player.util.UtilPlayer;
import com.envyful.api.platform.PlatformProxy;
import com.envyful.api.time.UtilTime;
import com.envyful.api.time.UtilTimeFormat;
import com.envyful.gts.forge.EnvyGTSForge;
Expand Down Expand Up @@ -34,11 +35,10 @@ public static void openUI(ForgeEnvyPlayer player, double time, boolean error) {
.replace("%max_duration%", UtilTimeFormat.getFormattedDuration(
TimeUnit.SECONDS.toMillis(EnvyGTSForge.getConfig().getMinTradeDuration())
))
.replace("%item%", itemInHand.getHoverName().getString())))
.replace("%pokemonItem%", itemInHand.getHoverName().getString())))
Copy link
Member

@danorris709 danorris709 Jul 15, 2025

Choose a reason for hiding this comment

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

Same question here, regarding what was the reasoning for changing this?

.showInput()
.defaultText(TimeUnit.SECONDS.toMinutes(EnvyGTSForge.getConfig().getMinTradeDuration()) + "m")
.closeOnEscape()
.onClose(closedScreen -> SellHandOrParty.open(player))
.buttons(DialogueButton.builder()
.text("Submit")
.onClick(submitted -> {
Expand All @@ -55,6 +55,7 @@ public static void openUI(ForgeEnvyPlayer player, double time, boolean error) {
}

UtilPlayer.runCommand(player.getParent(), "gts sell " + itemInHand.getCount() + " " + time + " " + submitted.getInput());
PlatformProxy.runSync(player.getParent()::closeContainer);
Copy link
Member

@danorris709 danorris709 Jul 15, 2025

Choose a reason for hiding this comment

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

Same again here, clicking the button should be closing the UI so why is this here?

})
.build())
.sendTo(player.getParent()), 5);
Expand Down
2 changes: 0 additions & 2 deletions neo21/src/main/java/com/envyful/gts/forge/ui/SelectPartyPokemonUI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@ private static void setPokemon(ForgeEnvyPlayer player, Pane pane) {
item.enchant(enchants.getHolder(Enchantments.MENDING).orElseThrow(), 1);
}

player.getParent().getInventory().add(item.copy());

pane.set(pos % 9, pos / 9, GuiFactory.displayableBuilder(ItemStack.class)
.itemStack(item)
.clickHandler((envyPlayer, clickType) -> {
Expand Down