new InventoryList class system #119

Henrybk · 2016-06-15T21:36:46Z

Code review
QA testing

This is a big code review and rework that focus on getting cart, storage and character inventory to be managed in the same way, it seems to work well. I added support for macro plugin but haven't tested it yet.

I have done no work with the Wx interface, so if someone likes this idea and wants to help, feel free.

…inventory_class_system

Henrybk · 2016-10-11T19:22:25Z

In the last commit I fixed a bug with the line ending character, now it should be way better to check the code changes.

It would be of great help it this could be tested, I'll try later to comment on all fixed bugs.

farrainbow

Overall this is very good and should be merged ASAP to not cause many conflicts later. I can see many further improvements to this but they can be done later.

Things I want to discuss before merging:

farrainbow · 2016-12-05T14:44:23Z

src/Actor/You.pm

+# Get the storage list for this character.
+sub storage {
+	return $_[0]->{__storage};
+}


Do we really need an indirection there or it's just for writing $char->storage->... instead of $char->{storage}->...?

It was just done to be able to use $char->storage->, I don't think directly accessing these variables is a good practice.

farrainbow · 2016-12-05T14:52:22Z

src/InventoryList/Cart.pm

+sub weight {
+	my ($self) = @_;
+	return $self->{weight};
+}


Comment about misdirection applies there as well and in all similar places. Maybe keep it simple?

This one I agree is a little too much misdirection.

farrainbow · 2016-12-05T14:54:55Z

src/InventoryList/Storage.pm

+	return $self->{openedThisSession} == 1;
+}
+
+sub isOpened {


Maybe name it isReady, as in Inventory and Cart if they all have the same purpose?

Same as above, I'll review this later today.

farrainbow · 2016-12-05T14:56:17Z

src/InventoryList/Cart.pm

+	$self->clear();
+}
+
+sub release {


Maybe name it close as in Storage if it has the same purpose? Common API is good to have.

I will review this later today, you are probably right but I am not home right now to be able to properly consider it.

Henrybk · 2016-12-05T15:12:31Z

@farrainbow These are the changes I would like the most to see implemented, thanks for helping me and reviewing my code :)

* fix for autostorage bug When kore puts an item into storage but keeps some of it in inventory we receive the error message "Unable to store", but the item was stored, this is due to an error in the error message verification. this change should fix it. * Moved login_error and login_error_game_login_server * Moved character_deletion_successful * Moved character_deletion_failed * Moved character_moves * Moved character_name, character_status and chat_created * Moved chat_info * Moved 9 more functions * Moved 4 more functions * Moved egg_list * Moved emoticon * Moved 3 more functions * Moved 5 more functions * Moved 3 guild functions * Moved 8 more functions * Moved 4 more functions * Moved 2 more functions * Move 4 more functions * Moved 3 more functions * Fixed a function call, added constants to receive.pm and moved 1 function * Moved 3 more functions * support packet 0441 (skill_delete) * Support for 02F1 Adds support for 02F1 (PACKET_CZ_PROGRESS), packet which notifies the server that the progress bar has been completed * update iRO files * 02F1 [Sakexe_0] * Poseidon compatibility with bRO Adds missing charblocksizes to poseidon server Adds bRO_2016-10-11a poseidon servertype Adds 09D0 as a gameguard sync packet * Update idRO Items for MT Oct 11, 2016. Signed-off-by: Cydh Ramdh <cydh@pservero.com> * [2016-10-11] bRO Connection Files * storage_gettocart changes Check if cart exists before send item straight from storage to cart ! * storage_gettocart changes Check if cart exists before send item straight from storage to inventory. * update iRO files * update iRO files * bRO official files [June to October] * bRO's skillnametable from skillinfolist.lua Updates bRO's skillnametable from its decompiled skillinfolist.lub (path in data.grf: data\luafiles514\lua files\skillinfoz\skillinfolist.lub) * Fix formatting for NPC talk response choices OpenKore#271 Fixed two issues outlined in Ticket OpenKore#271: Issue A - Text for the NPC talk response choices were cut off if they were longer than 36 characters. Issue B - Numeric response codes for the choices need to be right justified because up to two digit codes (i.e. more than 0-9 choices) are possible. Fixed by referencing: http://perldoc.perl.org/perlform.html ticket: OpenKore#271 branch: talk_response modified: src/Commands.pm * Adds 00A7 and 00BB These packets are sent to the server by the client. 00A7 -> CZ_USE_ITEM 00BB -> CZ_STATUS_CHANGE * changes at bRO portals.txt Disabled old gef_fild and updated all Eden Officers NPC locations * Correction of Gonryun Eden Officer NPC location * bRO connection files + poseidon servertype * Reverted NPC responses to left alignment * update iRO files * Now only files that match the regex will be hooked * storage_gettocart changes Check cart using $char->cartActive, before storage item to cart. * storage_gettocart changes 2 Check cart using $char->cartActive, before storage item to cart. * update iRO files * fix OpenKore#269 - add more useful error when XKore2 has a port conflict * improve error message * Fixed error message for XKore 3 and updated translations * [2016-10-25] bRO Connection files + Poseidon servertype Weekly bRO update + latest Poseidon servertype. Removes old, unused Poseidon servertype The brazillian comunity has been asked and we decided to keep only the two latest clients in servertypes.txt * Fixes OpenKore#290 * Fix unintended crash caused by XKore 2 Unintended crash, reported by anarke and conkister. I can confirm it, just open openkore configured with XKore 2, with a valid IP/port and Kore will crash with no error message http://openkorebrasil.org/index.php?/topic/1701-xkore2-bug-estranho/ (pt-br) * Remove message written in Brazilian Portuguese and misleading link Removes message written in Brazilian Portuguese from MapServer.pm and broken, not necessary, link to an old Forum * update iRO files * update iRO files * update iRO files * Description for breakTime plugin Adds the description for the breakTime plugin (plugin help command) * [2016-11-01] Connection files for bRO * [2016-11-01] Poseidon servertype for bRO * Fixes OpenKore#305 * update iRO files * [2016-11-08] Connection Files for bRO http://openkorebrasil.org/index.php?/topic/1814-atualiza%C3%A7%C3%A3o-08-11-2016/ * [2016-08-11] Poseidon servertype for bRO added bRO_2016-11-08a removed bRO_2016-10-25a * Remove unused dependency This was used by the autoupdater tool and only by it, which is now deprecated. Removing this dependency makes no difference whatsoever. autoupdater source -> https://svn.code.sf.net/p/openkore/code/openkore/trunk/autoupdate.pl wiki article -> http://wiki.openkore.com/index.php/AutoUpdater * update iRO files * portals.txt changes Remove unaccessible (removed) maps since renewal (february, 2011) Change portals for maps which portals now connect to different maps: ein_fild01 -> ein_fild05, previously ein_fild02 umbala -> yggdrasil01, previously um_dun01 ein_fild01 -> cave * [2016-11-17] Connection Files for bRO * update iRO files * update iRO files * Add Route from Airport to Einbroch at bRO added the correct route from airport to einbroch at portals.txt * add new version of splendide map (with access to bif_fild01) * update iRO files * Added hook 'AI_start'

…inventory_class_system # Conflicts: # src/Network/Receive/ServerType0.pm # src/Network/Receive/kRO/Sakexe_0.pm

Henrybk · 2016-12-05T16:18:38Z

Wow I did some crazy wrong stuff but it's alright now.

Henrybk · 2016-12-07T09:15:33Z

I just remembered the problem with this pr, i never updated the wx interface code, i will have to see about that

farrainbow · 2016-12-07T09:33:30Z

I can do Wx stuff, maybe push this as a branch to the main repository?

…inventory_class_system

Henrybk · 2016-12-09T15:52:01Z

@farrainbow I have, in the last commit, implemented most of your suggestions.

farrainbow · 2016-12-09T16:09:37Z

storage->wasOpenedThisSession better explains what it really does, nice one.

farrainbow · 2016-12-11T14:02:27Z

Merge those: https://github.com/OpenKore/openkore/tree/new_inventory_class_system

…to new_inventory_class_system

Henrybk · 2016-12-11T15:00:22Z

@farrainbow Done

Henrybk · 2016-12-11T20:40:18Z

I believe this is now complete

Henrybk · 2016-12-11T21:28:09Z

Let the bugs come

v1.0

1d9a41f

itsrachelfish added needs to be tested needs code review labels Jun 16, 2016

Henrybk mentioned this pull request Oct 3, 2016

Merge pickupitems.txt with items_control.txt #227

Open

Henrybk added enhancement priority: medium labels Oct 9, 2016

Henrybk added 2 commits October 11, 2016 16:14

Fix bug with LF vs CR (line ending ascii)

653f979

Merge remote-tracking branch 'refs/remotes/OpenKore/master' into new_…

63d2370

…inventory_class_system

farrainbow added priority: urgent and removed priority: medium labels Dec 5, 2016

farrainbow reviewed Dec 5, 2016

View reviewed changes

Henrybk added 3 commits December 5, 2016 14:03

Merge remote-tracking branch 'refs/remotes/OpenKore/master' into new_…

0d1927d

…inventory_class_system # Conflicts: # src/Network/Receive/ServerType0.pm # src/Network/Receive/kRO/Sakexe_0.pm

Rrmove crazy stuff I didn't do on purpose

43ffb8b

Merge remote-tracking branch 'refs/remotes/OpenKore/master' into new_…

8b3fd79

…inventory_class_system

farrainbow mentioned this pull request Dec 8, 2016

Item-related code renames #350

Closed

9 tasks

Syntax changes proposed by eharvest

82b98b4

farrainbow added 2 commits December 11, 2016 16:52

Use $char->cart and $char->storage in Wx interface

1557dc0

Fix undefined variable in XKore 2

8981563

farrainbow and others added 2 commits December 11, 2016 17:26

Use $char->cart and $char->storage in webMonitor

89e2b16

Merge remote-tracking branch 'upstream/new_inventory_class_system' in…

d7cb517

…to new_inventory_class_system

farrainbow and others added 10 commits December 11, 2016 19:11

Fix syntax

7f91527

Use isReady for storage

0bb17f8

Fix indentation

ae8cba8

Use $char->cart instead of %cart

1d313ea

Removed dupped sub in receive.pm

ceff8f7

Remove acceses to $ai_v{'inventory_time'} in corelogic

dd23280

Added comments

549e5a4

Forgot this (sorry lol)

8ccce74

Fixed acess to %cart in Macro::Parser

7934056

Re-adding hooks removed by accident

b0fe26b

farrainbow merged commit 17b82ad into OpenKore:master Dec 11, 2016

Henrybk mentioned this pull request Dec 13, 2016

Managing inventory lists #37

Closed

Henrybk mentioned this pull request May 3, 2017

error when used cart & storage (package "Actor::You") #572

Closed

Henrybk deleted the new_inventory_class_system branch August 27, 2017 18:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new InventoryList class system #119

new InventoryList class system #119

Henrybk commented Jun 15, 2016 •

edited by farrainbow

Henrybk commented Oct 11, 2016

farrainbow left a comment

farrainbow Dec 5, 2016

Henrybk Dec 5, 2016 •

edited

farrainbow Dec 5, 2016 •

edited

Henrybk Dec 5, 2016

farrainbow Dec 5, 2016

Henrybk Dec 5, 2016

farrainbow Dec 5, 2016 •

edited

Henrybk Dec 5, 2016

Henrybk commented Dec 5, 2016

Henrybk commented Dec 5, 2016

Henrybk commented Dec 7, 2016

farrainbow commented Dec 7, 2016 •

edited

Henrybk commented Dec 9, 2016

farrainbow commented Dec 9, 2016 •

edited

farrainbow commented Dec 11, 2016

Henrybk commented Dec 11, 2016

Henrybk commented Dec 11, 2016

Henrybk commented Dec 11, 2016

new InventoryList class system #119

new InventoryList class system #119

Conversation

Henrybk commented Jun 15, 2016 • edited by farrainbow

Henrybk commented Oct 11, 2016

farrainbow left a comment

Choose a reason for hiding this comment

farrainbow Dec 5, 2016

Choose a reason for hiding this comment

Henrybk Dec 5, 2016 • edited

Choose a reason for hiding this comment

farrainbow Dec 5, 2016 • edited

Choose a reason for hiding this comment

Henrybk Dec 5, 2016

Choose a reason for hiding this comment

farrainbow Dec 5, 2016

Choose a reason for hiding this comment

Henrybk Dec 5, 2016

Choose a reason for hiding this comment

farrainbow Dec 5, 2016 • edited

Choose a reason for hiding this comment

Henrybk Dec 5, 2016

Choose a reason for hiding this comment

Henrybk commented Dec 5, 2016

Henrybk commented Dec 5, 2016

Henrybk commented Dec 7, 2016

farrainbow commented Dec 7, 2016 • edited

Henrybk commented Dec 9, 2016

farrainbow commented Dec 9, 2016 • edited

farrainbow commented Dec 11, 2016

Henrybk commented Dec 11, 2016

Henrybk commented Dec 11, 2016

Henrybk commented Dec 11, 2016

Henrybk commented Jun 15, 2016 •

edited by farrainbow

Henrybk Dec 5, 2016 •

edited

farrainbow Dec 5, 2016 •

edited

farrainbow Dec 5, 2016 •

edited

farrainbow commented Dec 7, 2016 •

edited

farrainbow commented Dec 9, 2016 •

edited