Skip to content

Conversation

@Matan-94
Copy link

@Matan-94 Matan-94 commented Oct 15, 2024

Description

1. players were able to open their shop from any map using packet.
added Store Remote Controller inventory & location checking.
Steps:
1.open merchant and go any map you like.
2. send this packet: 3B 00 01 0D 00 54 30 35 30 34 32 36 37 30 32 31 38 58 01 00 00 00
3. merchant opened

2. players were able to duplicate their merchant across FM rooms.
Steps:
1.open merchant regularly put item and leave the shop.
2. go to other room in fm.
4.send this packet 7B 00 00 05 04 00 6C 61 6C 61 00 01 00 76 C0 4C 00
5. put item and open shop.
6. merchant duplicated.

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have tested my changes
  • I have added unit tests that prove my changes work

Screenshots

@Matan-94 Matan-94 changed the title added Store Remote Controller exists checking in inventory fixed Hired Merchant duplication & Store Remote Controller inventory checking Oct 16, 2024
@nutnnut
Copy link

nutnnut commented Oct 17, 2024

I recommend re-formatting your spaces formatting using intellij ctrl+alt+L and always put curly braces around conditions even if there is only 1 line of code inside to be consistent with the rest of the project.

Copy link
Owner

@P0nk P0nk left a comment

Choose a reason for hiding this comment

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

Good fixes, just need some minor changes.

chr.setHiredMerchant(merchant);
c.getWorldServer().registerHiredMerchant(merchant);
chr.sendPacket(PacketCreator.getHiredMerchant(chr, merchant, true));
System.out.println("new shop creation.");
Copy link
Owner

Choose a reason for hiding this comment

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

Remove println. Should use Slf4j logger if something needs to be logged. This doesn't need it though.


private final Lock lock = new ReentrantLock(true);;
private final Lock lock = new ReentrantLock(true);
;
Copy link

Choose a reason for hiding this comment

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

remove this semi colon

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.

5 participants