apex/DimDoors
1
1
Fork 0
Code Issues 26 Pull Requests Actions Packages Projects Releases Wiki Activity

Commands are guaranteed to be ICommand, not CommandBase. #140

Closed
CannibalVox wants to merge 1 commits from master into master
pull from: master
merge into: apex:master
Conversation 5 Commits 1 Files Changed 1 +4248 -13532
CannibalVox commented 2014-03-06 18:17:53 +00:00 (Migrated from github.com)
Copy Link

Currently some ChickenBones mods add commands that are based on ICommand but not CommandBase. This causes a crash when a DimDoors command is compared to those commands. The correct assumption is that all commands are ICommand, not CommandBase.

Currently some ChickenBones mods add commands that are based on ICommand but not CommandBase. This causes a crash when a DimDoors command is compared to those commands. The correct assumption is that all commands are ICommand, not CommandBase.
SenseiKiwi commented 2014-03-06 19:21:50 +00:00 (Migrated from github.com)
Copy Link

That's odd. Why are we even performing that comparison in the first place? Steven must have included it for a reason. For the time being, I've asked him to merge the PR ASAP.

That's odd. Why are we even performing that comparison in the first place? Steven must have included it for a reason. For the time being, I've asked him to merge the PR ASAP.
CannibalVox commented 2014-03-06 19:22:31 +00:00 (Migrated from github.com)
Copy Link

I'm not sure it's DD that's performing the comparison but obviously performing it is a valid thing to do.

I'm not sure it's DD that's performing the comparison but obviously performing it is a valid thing to do.
SenseiKiwi commented 2014-03-06 23:19:39 +00:00 (Migrated from github.com)
Copy Link

Another user told me about what was going on and I realized why the comparisons are being performed. The comparisons are done by the help command to sort commands alphabetically for displaying. If you think about it, though, then that also means that support for that comparison must exist in CommandBase itself. Perhaps Steven added the comparison because at some point when our commands implemented ICommand and not CommandBase? I'm not sure. But the point is that the function is unnecessary - CommandBase already handles that for us. We can just remove it from our code.

Another user told me about what was going on and I realized why the comparisons are being performed. The comparisons are done by the help command to sort commands alphabetically for displaying. If you think about it, though, then that also means that support for that comparison must exist in CommandBase itself. Perhaps Steven added the comparison because at some point when our commands implemented ICommand and not CommandBase? I'm not sure. But the point is that the function is unnecessary - CommandBase already handles that for us. We can just remove it from our code.
SenseiKiwi commented 2014-03-07 01:33:42 +00:00 (Migrated from github.com)
Copy Link

Fixed it in a separate commit along with some other stuff. No need for this PR anymore.

Fixed it in a separate commit along with some other stuff. No need for this PR anymore.
CannibalVox commented 2014-03-07 01:41:32 +00:00 (Migrated from github.com)
Copy Link

Sounds rad- thanks a lot!

Sounds rad- thanks a lot!

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
duplicate
enhancement
invalid
question
wontfix
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
apex
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: apex/DimDoors#140
No description provided.
Delete Branch "master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?