Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HUSS #3302

Closed
wants to merge 3 commits into from
Closed

HUSS #3302

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion conf/groups.conf
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,6 @@ groups: (
jail: true
jailfor: true
mute: true
storagelist: true
Copy link
Member

Choose a reason for hiding this comment

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

is the removal of storagelist permission and @storagelist atcommand intentional? I did not see this mention on the PR nor commit message about why this atcommand is being removed

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, this PR is just #2724 rebased, so it has the same caveats as the original. Which includes no default storage and not having implemented some conveniences like @storagelist command. Do keep in mind this got the point of "no changes requested by team" before being entirely forgotten by said team and rendered un-mergeable. Most issues you pointed out either weren't considered an issue in the original or are just maintenance upkeep. Large, complex, non-urgent PRs like this one suffer from an unreasonably large maintenance upkeep. Usually something with only two or one or none of these three attributes get in, breaking the PR and thus the high maintenance upkeep. Honorable mention to #466 which is another good example of death by upkeep.

...Although for these design decisions you can ask @dastgirp as they are the original author, I guess? I don't think they'll remember much though, it has been four years already.

(To be more clear and if memory serves as it's been a long time since I looked at the original, @storagelist will not work with this PR short of a total rewrite of the command, so it was decided to do a regression and remove the command, as it's seldom used and out of scope, from the discussions if it should list all storages or take an argument, up to the actual implementation which would add even more stuff to review in an already large PR. There was no objection from the original reviewers).

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, this PR is just #2724 rebased, so it has the same caveats as the original. Which includes no default storage and not having implemented some conveniences like @storagelist command. Do keep in mind this got the point of "no changes requested by team" before being entirely forgotten by said team and rendered un-mergeable. Most issues you pointed out either weren't considered an issue in the original or are just maintenance upkeep. Large, complex, non-urgent PRs like this one suffer from an unreasonably large maintenance upkeep. Usually something with only two or one or none of these three attributes get in, breaking the PR and thus the high maintenance upkeep. Honorable mention to #466 which is another good example of death by upkeep.

...Although for these design decisions you can ask @dastgirp as they are the original author, I guess? I don't think they'll remember much though, it has been four years already.

(To be more clear and if memory serves as it's been a long time since I looked at the original, @storagelist will not work with this PR short of a total rewrite of the command, so it was decided to do a regression and remove the command, as it's seldom used and out of scope, from the discussions if it should list all storages or take an argument, up to the actual implementation which would add even more stuff to review in an already large PR. There was no objection from the original reviewers).

cartlist: true
itemlist: true
stats: true
Expand Down
52 changes: 52 additions & 0 deletions conf/map/storage.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//================= Hercules Database ==========================================
//= _ _ _
//= | | | | | |
//= | |_| | ___ _ __ ___ _ _| | ___ ___
//= | _ |/ _ \ '__/ __| | | | |/ _ \/ __|
//= | | | | __/ | | (__| |_| | | __/\__ \
//= \_| |_/\___|_| \___|\__,_|_|\___||___/
//================= License ====================================================
//= This file is part of Hercules.
//= http://herc.ws - http://github.com/HerculesWS/Hercules
//=
//= Copyright (C) 2014-2024 Hercules Dev Team
//= Copyright (C) 2017 Smokexyz
//=
//= Hercules is free software: you can redistribute it and/or modify
//= it under the terms of the GNU General Public License as published by
//= the Free Software Foundation, either version 3 of the License, or
//= (at your option) any later version.
//=
//= This program is distributed in the hope that it will be useful,
//= but WITHOUT ANY WARRANTY; without even the implied warranty of
//= MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
//= GNU General Public License for more details.
//=
//= You should have received a copy of the GNU General Public License
//= along with this program. If not, see <http://www.gnu.org/licenses/>.
//==============================================================================
//= Storage Configuration
//==============================================================================
//= @Format Notes:
//= - There can be an unlimited amount of storages and limits.
//= - All setting names are case-sensitive and must be keyed accurately.
//= - It is not recommended to set the maximum storage capacity over 1000 items.

storage_conf: (
/******************************************************************************
********************************* Entry structure *****************************
*******************************************************************************
{
Id: (int) (required|unique) Unique Identifier
Name: (string) (required) Name of the storage sent to the client.
Capacity: (int) (required) Maximum capacity of the storage.
}
*******************************************************************************/
{
// DO NOT EDIT THIS ENTRY!
Copy link
Member

Choose a reason for hiding this comment

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

indent issue here

// This is the default (official) storage for an account.
Id: 1
Name: "Storage"
Capacity: 600
},
)
Copy link
Member

Choose a reason for hiding this comment

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

Please add an empty line at the end of the file, this makes it friendlier for diffs/version control systems.

10 changes: 7 additions & 3 deletions conf/messages.conf
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
24: Job level raised.
25: Job level lowered.
26: [%d] seconds left until you can use
27: Storage has been not loaded yet.
27: Storage '%s' has not been loaded yet.
28: No player found.
29: 1 player found.
30: %d players found.
Expand Down Expand Up @@ -91,7 +91,11 @@
62: Judgement has passed.
63: Mercy has been shown.
64: Mercy has been granted.
//65-69 FREE
65: Please specify a storage name. (usage: @storeall <storage name/ID>).
66: Please specify a storage name. (usage: @clearstorage <storage name/ID>).
67: Invalid storage name or ID
68: Please specify a storage name. (usage: @storage <storage name/ID>).
//69 FREE
70: You have learned the skill.
71: You have forgotten the skill.
72: War of Emperium has been initiated.
Expand Down Expand Up @@ -779,7 +783,7 @@
918: Please enter a speed value (usage: @speed <%d-%d>).

// @storage
919: Storage opened.
919: Storage #%d opened.

// @guildstorage
920: Guild storage opened.
Expand Down
3 changes: 3 additions & 0 deletions db/constants.conf
Original file line number Diff line number Diff line change
Expand Up @@ -4923,4 +4923,7 @@ constants_db: {
SI_SP_SHA: 1064
SI_SOULCURSE: 1125
SI_MADOGEAR: 1149

comment__: "Storage Types"
STORAGE_TYPE_MAIN: 1
}
12 changes: 10 additions & 2 deletions doc/script_commands.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6164,7 +6164,9 @@ has no gear, script will be terminated with an error.
//=====================================
---------------------------------------

*openstorage()
*openstorage(<storage_id>{, <storage_mode>})
Copy link
Member

Choose a reason for hiding this comment

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

splitting access logic and id logic into separate commits would be better


Default Storage Access Mode: STORAGE_ACCESS_ALL

This will open character's Kafra storage window on the client connected to
the invoking character. It can be used from any kind of NPC or item
Expand All @@ -6176,13 +6178,19 @@ storage window, to avoid any disruption when both windows overlap.

mes("I will now open your stash for you");
close2();
openstorage();
openstorage(STORAGE_TYPE_MAIN);
end;

The mapflag 'nostorage' when set to type '2' (or 3), will not open the
account storage. Unless the character group has the permission 'bypass_nostorage'.
In case blocked by mapflag, returns 0.

Storage Modes:
STORAGE_ACCESS_VIEW // View storage only
STORAGE_ACCESS_GET // Allow getting items from storage.
STORAGE_ACCESS_PUT // Allow putting items to storage.
STORAGE_ACCESS_ALL // Allow all actions.

---------------------------------------

*openmail()
Expand Down
2 changes: 1 addition & 1 deletion npc/custom/etc/quest_warper.txt
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ L_Storage:
}
mes "Close this window and I will open your storage.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;

L_StorageJBlow:
Expand Down
2 changes: 1 addition & 1 deletion npc/kafras/functions_kafras.txt
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ function script F_KafStor {
}
callfunc("F_CheckKafCode"); //check your storage password, if set
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
cutin "", 255;
end;
}
Expand Down
2 changes: 1 addition & 1 deletion npc/kafras/kafras.txt
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ aldeba_in,96,181,4 script Kafra Service 4_F_KAFRA5,{
mes "Thank you for your patronage.";
callfunc("F_CheckKafCode"); //check your storage password, if set
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
break;
case 3:
mes "[Kafra Leilah]";
Expand Down
2 changes: 1 addition & 1 deletion npc/other/CashShop_Functions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function script F_CashStore {
mes "Welcome to the Kafra Corporation.";
mes "Here, let me open your Storage for you.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
cutin "",255;
return;
}
Expand Down
6 changes: 3 additions & 3 deletions npc/quests/quests_13_1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7686,7 +7686,7 @@ function Catwarp;
mes "Your storage will";
mes "be opened shortly.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}
else {
Expand Down Expand Up @@ -7733,7 +7733,7 @@ function Catwarp;
mes "Your storage will";
mes "be opened shortly.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}
else {
Expand Down Expand Up @@ -7855,7 +7855,7 @@ function Catwarp;
mes "Your storage will";
mes "be opened shortly.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}
else {
Expand Down
6 changes: 3 additions & 3 deletions npc/quests/quests_13_2.txt
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ spl_fild02,25,211,4 script Cat Hand Agent#spl 4_M_BOSSCAT,{
mes "Thank you.";
mes "Your storage will be opened shortly.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}
else {
Expand Down Expand Up @@ -156,7 +156,7 @@ spl_fild02,25,211,4 script Cat Hand Agent#spl 4_M_BOSSCAT,{
mes "Thank you.";
mes "Your storage will be opened shortly.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}
else {
Expand Down Expand Up @@ -277,7 +277,7 @@ spl_fild02,25,211,4 script Cat Hand Agent#spl 4_M_BOSSCAT,{
mes "Thank you.";
mes "Your storage will be opened shortly.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}
else {
Expand Down
2 changes: 1 addition & 1 deletion npc/re/cities/mora.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1194,7 +1194,7 @@ mora,48,128,0 script Drawer#mora_warehouse HIDDEN_NPC,{
close;
}
Zeny -= 100;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
close;
case 2:
mes "- You gave up using the warehouse. -";
Expand Down
2 changes: 1 addition & 1 deletion npc/re/jobs/2e/rebellion.txt
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,7 @@ job_gun,216,138,4 script Butler Karlex 1_M_01,{
mes("Thank you for using the service.");
close2();
if (.@select == 2) {
openstorage();
openstorage(STORAGE_TYPE_MAIN);
}
end;
}
Expand Down
2 changes: 1 addition & 1 deletion npc/re/jobs/novice/academy.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5422,7 +5422,7 @@ iz_ac01,95,46,5 script Kafra Guide Trainer#ac 4_F_KAFRA1,{
mes("Thanks for using~!");
close2();
cutin("", 255);
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
} else {
mes("[Kafra Guide Trainer]");
Expand Down
2 changes: 1 addition & 1 deletion npc/re/other/dimensional_gap.txt
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ dali,112,95,4 script Logistics Manager 4_M_MERCAT2,{
mes("You need at least 200 zeny to use the Storage.");
} else {
Zeny -= 200;
openstorage();
openstorage(STORAGE_TYPE_MAIN);
}
close();
}
Expand Down
2 changes: 1 addition & 1 deletion npc/re/quests/eden/eden_service.txt
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
}
Zeny -= 500;
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}
mes "I don't have enough zeny.";
Expand Down
2 changes: 1 addition & 1 deletion npc/re/quests/quests_dicastes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ dic_in01,254,119,0 script Item Storage#01 CLEAR_NPC,{
}
Zeny -= 500;
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}

Expand Down
2 changes: 1 addition & 1 deletion npc/re/quests/quests_malangdo.txt
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ malangdo,184,139,4 script Storekeeper#mal 4_CAT_ADV2,{
mes "[Storekeeper]";
mes "Thank you.";
close2;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
}

Expand Down
2 changes: 1 addition & 1 deletion npc/woe-fe/agit_main.txt
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ OnRecvCastle:
mes "the Kafra Service.";
close2;
cutin "",255;
openstorage;
openstorage(STORAGE_TYPE_MAIN);
end;
case 2:
mes "[Kafra Employee]";
Expand Down
2 changes: 1 addition & 1 deletion npc/woe-se/agit_main_se.txt
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ OnInit:
mes("have at least Novice Skill");
mes("Lv.6 to use the Storage.");
}
else openstorage;
else openstorage(STORAGE_TYPE_MAIN);
break;
case 2:
mes("[Kafra Employee]");
Expand Down
2 changes: 2 additions & 0 deletions sql-files/main.sql
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1599908598); -- 2020-09-1
INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1613840320); -- 2021-02-20--19-57.sql
INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1618058468); -- 2021-04-10--15-36.sql
INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1653155461); -- 2022-05-21--29-49.sql
INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1718685120); -- 2024-06-18--12-32.sql

--
-- Table structure for table `storage`
Expand All @@ -975,6 +976,7 @@ INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1653155461); -- 2022-05-2
CREATE TABLE IF NOT EXISTS `storage` (
`id` INT UNSIGNED NOT NULL AUTO_INCREMENT,
`account_id` INT UNSIGNED NOT NULL DEFAULT '0',
`storage_id` INT UNSIGNED NOT NULL DEFAULT '1',
`nameid` INT UNSIGNED NOT NULL DEFAULT '0',
`amount` SMALLINT UNSIGNED NOT NULL DEFAULT '0',
`equip` INT UNSIGNED NOT NULL DEFAULT '0',
Expand Down
23 changes: 23 additions & 0 deletions sql-files/upgrades/2024-06-18--12-32.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#1718685120

-- This file is part of Hercules.
-- http://herc.ws - http://github.com/HerculesWS/Hercules
--
-- Copyright (C) 2013-2024 Hercules Dev Team
--
-- Hercules is free software: you can redistribute it and/or modify
-- it under the terms of the GNU General Public License as published by
-- the Free Software Foundation, either version 3 of the License, or
-- (at your option) any later version.
--
-- This program is distributed in the hope that it will be useful,
-- but WITHOUT ANY WARRANTY; without even the implied warranty of
-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-- GNU General Public License for more details.
--
-- You should have received a copy of the GNU General Public License
-- along with this program. If not, see <http://www.gnu.org/licenses/>.

ALTER TABLE `storage` ADD `storage_id` INT UNSIGNED NOT NULL DEFAULT '1' AFTER `account_id`;

INSERT INTO `sql_updates` (`timestamp`) VALUES (1718685120);
Copy link
Member

Choose a reason for hiding this comment

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

please add an empty line at the end of the file

1 change: 1 addition & 0 deletions sql-files/upgrades/index.txt
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,4 @@
2022-01-05--19-00.sql
2022-10-08--08-35.sql
2022-05-21--29-49.sql
2024-06-18--12-32.sql
Loading
Loading