nekiro / TFS-1.5-Downgrades

Alternative forgottenserver versions for older protocols support

72 stars 83 forks source link

adding compatibility to 772 otbm version (version = 1) #119

Closed danilopucci closed 2 years ago

danilopucci commented 2 years ago

Pull Request Prelude

[x] I have followed proper The Forgotten Server code styling.
[x] I have read and understood the contribution guidelines before making this PR.
[x] I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Protocol version 7.72

Changes Proposed

This pull-request adds retrocompatibility to 7.6 .otbm map version (version = 1). The changes are basically reading properly the count variable after splashes/fluidcontainers and stackables when the otbm version = 1. Also, adds the reading os ITEM_GROUP on switch-case statement while reading items.otb

Issues addressed:

118

gesior commented 2 years ago

Code looks fine. I don't have any 7.6 map to check, if it works. @danilopucci

PR and commit names are misleading. We add compatibility for version 0, not 1, right?
Have you tested, if it works fine with runes with charges inside ex. chest/box? On old engines I often see .isRune() check next to stackable.
```
if (iType.stackable || iType.isSplash() || iType.isFluidContainer()) {
```

danilopucci commented 2 years ago

@gesior 1 - Yeah, it is a bit confusing. On my titles I mean about OTBM version 1 (header=0). This link is a coding reference and this other one is a usage reference 2 - Yes, I tested with runes on floor and after your comment I put a rune inside a chest. I made a search about the isRune() but I didnt find anything. The closer I found was a checkage of isCharged() but it is on Item Attributes and for client version > 820. According to this RME reference, the runes charges are not going to be saved as this.

I am attaching the map that I tested and made the runes/chest modifications map_test.zip

nekiro commented 2 years ago

I'll merge this, what can happen right?