issue/finish-factions-and-banking #2

Merged
jkibbels merged 11 commits from issue/finish-factions-and-banking into dev 2025-02-01 19:26:22 +00:00
Owner

This is all the work from factions and banking that has been done over the past couple months. Making a pull request now and will respectively put the rest of the work on their own branches because this is finally in its own comfortable area

This is all the work from factions and banking that has been done over the past couple months. Making a pull request now and will respectively put the rest of the work on their own branches because this is finally in its own comfortable area
walrus was assigned by jkibbels 2025-01-27 03:20:56 +00:00
jkibbels added 10 commits 2025-01-27 03:20:57 +00:00
[factions-banking] Correct faction block GUI draw slot positions with new gui
Some checks are pending
build / build (21) (push) Waiting to run
7d5fbca603
[factions-banking] Added faction attachment to block. Still a bit buggy
Some checks are pending
build / build (21) (push) Waiting to run
fa5bc741ec
[factions-banking] More updates
Some checks failed
build / build (21) (push) Has been cancelled
bf3b1f6ed2
[factions-banking] Final commit for this branch that has grown out of control. The other stuff will be added in their own respective branches
Some checks failed
build / build (21) (push) Has been cancelled
build / build (21) (pull_request) Has been cancelled
e164a489b2
jkibbels self-assigned this 2025-01-27 03:21:28 +00:00
walrus was unassigned by jkibbels 2025-01-27 03:21:29 +00:00
jkibbels requested review from walrus 2025-01-27 03:21:38 +00:00
Author
Owner

This is technically only a hair portion of this branch by the way - I force merged dev with this branch changes to get @walrus going on their own branch for faction beacon stuff. it's my repo so it's whatever, but the changes relevant in this code review have been changes done over the past week or so

This is technically only a hair portion of this branch by the way - I force merged dev with this branch changes to get @walrus going on their own branch for faction beacon stuff. it's my repo so it's whatever, but the changes relevant in this code review have been changes done over the past week or so
walrus requested changes 2025-01-28 21:37:59 +00:00
walrus left a comment
Collaborator

This is a long change

This is a long change
@ -33,1 +40,3 @@
private void ApplyEffects(ServerPlayerEntity player) {
private Integer GetBeaconAmplifier(FactionTierEnum tier) {
switch (tier) {
case TIER_I:
Collaborator

you should leave a commet saying TIER_I has no effects this could be confusing

you should leave a commet saying TIER_I has no effects this could be confusing
@ -118,4 +139,2 @@
///
/// @param[in] The new default account global account identifier
///
/// @return Changes the players default account at a selected bank
Collaborator

replace this doxygen part?

replace this doxygen part?
jkibbels marked this conversation as resolved
@ -68,0 +73,4 @@
if (!playerSelectedBank.isEmpty()) {
IndividualBank bank = GetBankByName(playerSelectedBank);
Optional<Integer> amount = bank.GetAccountBalance(AccountNumberGenerator.GetAccountNumberFromId(playerConfigs.get(player.getUuidAsString()).defaultSelectedAccount));
if (amount.isPresent()) {
Collaborator

if this actually works id be impressed with it >_>

if this actually works id be impressed with it >_>
jkibbels marked this conversation as resolved
@ -145,2 +147,2 @@
Integer remaining = accountBalance - amount;
success = (remaining < 0 && !allowNegativeBalance);
int remaining = accountBalance - amount;
success = (remaining >= 0 || allowNegativeBalance);
Collaborator

is allowNegativeBalance a method?

is allowNegativeBalance a method?
jkibbels marked this conversation as resolved
@ -415,4 +410,0 @@
/// @param[in] newAlias is name to give this account
///
/// @brief Alias an account
/////////////////////////////////////////////////////////////////////////////
Collaborator

smeems wrong to get rid of this Doxygen and replace it with in code comments are we not using doxygen?

smeems wrong to get rid of this Doxygen and replace it with in code comments are we not using doxygen?
@ -4,3 +3,1 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.*;
Collaborator

is this actually nessesary? or is this alot of extra imports IE:https://www.geeksforgeeks.org/java-util-package-java/

is this actually nessesary? or is this alot of extra imports IE:https://www.geeksforgeeks.org/java-util-package-java/
@ -37,3 +35,3 @@
// KBIC will ensure an amount of money based on its trustworthiness to a bank and the number of holders it has.
private Integer kbicInsuredAmount = 0;
private Integer kbicInsuredAmount = 75_000;
private Boolean kbicInsured = false;
Collaborator

maaaaaaan is it boolean or Boolean

maaaaaaan is it boolean or Boolean
Author
Owner

both

both
@ -36,3 +27,1 @@
case GREEN:
return colorStr + "2";
}
return switch (code) {
Collaborator

:O return switch is awesome

:O return switch is awesome
jkibbels marked this conversation as resolved
@ -39,2 +32,4 @@
case GREEN -> colorStr + "2";
};
// If this code is reachable, then someone has not properly handled the above switch-case
Collaborator

no code here and also maybe use a default case: if your worried about it

no code here and also maybe use a default case: if your worried about it
jkibbels marked this conversation as resolved
@ -42,6 +55,57 @@ public class ChatMenu {
this.rightArrow = rightArrow;
}
private Text MakeFooter() {
Collaborator

Footnote?

Footnote?
Author
Owner

? This is for making the footer in the message sent to the user

? This is for making the footer in the message sent to the user
jkibbels marked this conversation as resolved
@ -409,4 +424,0 @@
///
/// @return True if player is operator, false if not
/////////////////////////////////////////////////////////////////////////////
public Boolean IsOperator(ServerPlayerEntity player) {
Collaborator

good choice to remove this is a wierd function

good choice to remove this is a wierd function
jkibbels marked this conversation as resolved
@ -22,2 +22,2 @@
BlockManager.RegisterBlock("example_statue", new Block(FabricBlockSettings.copyOf(Blocks.BELL)));
BlockManager.RegisterBlock("faction_base_block", new FactionBaseBlock(FabricBlockSettings.copyOf(Blocks.IRON_BLOCK).nonOpaque()));
BlockManager.RegisterBlock("example_statue", new Block(FabricBlockSettings.copyOf(Blocks.BELL).nonOpaque()));
BlockManager.RegisterBlock("faction_base_block", new FactionBaseBlock(FabricBlockSettings.copyOf(Blocks.COBBLESTONE).requiresTool().nonOpaque()));
Collaborator

did nonOpaque fix the faces not rendering issue of blocks ajacent

did nonOpaque fix the faces not rendering issue of blocks ajacent
Author
Owner

yes

yes
jkibbels marked this conversation as resolved
jkibbels added 1 commit 2025-02-01 19:25:51 +00:00
[factions-banking] Start of shop keeper stuff. Not working - but on wrong branch. Oh well!
Some checks failed
build / build (21) (push) Has been cancelled
build / build (21) (pull_request) Has been cancelled
415983edca
jkibbels merged commit 64236d94b6 into dev 2025-02-01 19:26:22 +00:00
Author
Owner

This PR is closed and merged.

This PR is closed and merged.
jkibbels locked as Resolved and limited conversation to collaborators 2025-02-01 19:26:43 +00:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: jkibbels/the_big_one#2
No description provided.