openrs2
/
openrs2
3
1
Fork 3
Code Issues 68 Pull Requests Releases Activity
Labels Milestones

Flip > and >= in for loop conditions to < and <= respectively #48

Closed
Desetude wants to merge 1 commits from Desetude:feature/for-loop-condition-flip into master
Conversation 1 Commits 1 Files Changed 2
Desetude commented 4 years ago

Here's a diff of the client produced on my branch and the one produced on master: https://gist.github.com/Desetude/1453e2df162cd508ea5f6a1da89058dc.
Resolves bullet point 6 of #7.

Here's a diff of the client produced on my branch and the one produced on master: https://gist.github.com/Desetude/1453e2df162cd508ea5f6a1da89058dc. Resolves bullet point 6 of #7.
gpe requested changes 4 years ago
deob-ast/src/main/java/dev/openrs2/deob/ast/transform/ForLoopConditionTransformer.kt Outdated
class ForLoopConditionTransformer : Transformer() {
override fun transform(unit: CompilationUnit) {
unit.walk(Node.TreeTraversal.POSTORDER) { stmt: ForStmt ->
if (stmt.compare.isEmpty) {
gpe commented 4 years ago
Owner

isEmpty doesn't seem to be a function/property on Optional for me, so this doesn't build for me. I don't know if that's some sort of Kotlin set up problem on my end though.

I think I might also prefer using .ifPresent() with a lambda rather than separate isPresent() and get() calls, as it's then guaranteed that you can't forget the isPresent(). Some of the other transformers do this.

`isEmpty` doesn't seem to be a function/property on `Optional` for me, so this doesn't build for me. I don't know if that's some sort of Kotlin set up problem on my end though. I think I might also prefer using `.ifPresent()` with a lambda rather than separate `isPresent()` and `get()` calls, as it's then guaranteed that you can't forget the `isPresent()`. Some of the other transformers do this.
deob-ast/src/main/java/dev/openrs2/deob/ast/transform/ForLoopConditionTransformer.kt Outdated
else -> return@walk
}
stmt.setCompare(BinaryExpr(expr.right, expr.left, flipped))
gpe commented 4 years ago
Owner

Does stmt.compare = ... work here instead? I think that'd be the preferred Kotlin style, if so.

Does `stmt.compare = ...` work here instead? I think that'd be the preferred Kotlin style, if so.
Desetude commented 4 years ago
Poster

I think it doesn't work.

I think it doesn't work.
gpe approved these changes 4 years ago
gpe commented 4 years ago
Owner

cherry-picked, thanks!

cherry-picked, thanks!
gpe closed this pull request 4 years ago
Desetude deleted branch feature/for-loop-condition-flip 4 years ago
This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
Labels
Apply labels
Clear labels
archive
Related to the archiving service asm
Related to the ASM utility module buffer
Related to the buffer library bug
A bug build
Related to the build infrastructure cache
Related to the cache library compression
Related to the compression module crypto
Related to the cryptography module deobfuscator
Related to deobfuscation (at both the bytecode- and AST-level) or decompilation docs
Related to the documentation stored in the repository feature
A new feature improvement
An improvement to an existing feature, such as refactoring, documenting or adding tests patcher
Related to client patching protocol
Related to the protocol module research
Requires further research before progress can be made util
Related to the common utility module or the YAML module
No Label archive asm buffer bug build cache compression crypto deobfuscator docs feature improvement patcher protocol research util
Milestone
Set milestone
Clear milestone
No items
No Milestone
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: openrs2/openrs2#48
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'Desetude:feature/for-loop-condition-flip'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes