Initial release of Isle of Bios #3337

Vaans · 2025-02-11T02:01:53Z

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

This pull request introduces the initial release of the Isle of Bios instance, adding new NPCs, quests, items, and instance mechanics.

Changes Included

Added NPC_Comet for in-game interactions.
Integrated Quest Database entries related to the Isle of Bios.
Updated Item Database with relevant items for the instance.
Implemented the Instance mechanics, including spawn points and progression logic.

Issues Addressed

This PR resolves #241 , ensuring the instance is fully functional and integrated with the existing game mechanics.

Testing & Validation

Instance tested for proper enemy spawns and player interactions.
NPC dialogs and quest flow verified.
Items correctly registered in the database.
No breaking changes detected with existing content.

Additional Notes

Further balancing and improvements may be required based on feedback.
Any additional suggestions or testing feedback are welcome.
This is a conversion of the original files from rA with some modifications to make them work in Hercules.

guilherme-gm

Thank you for your contribution!

I finished a first review, I could see several formatting issues, but also some logical ones too.

I did not try running it, I can see it passes CI, but from my code review I believe some things are likely to fail at runtime (maybe under specific circumstances)

Some things I am not really sure, so there are several questions on my review. And anyways, feel free to ask/counter request changes that doesn't make sense. I don't know much about this quest, so I maybe have missed something

db/re/mob_skill_db.conf

db/re/item_group.conf

db/re/mob_db.conf

db/re/skill_db.conf

Vaans · 2025-03-13T23:29:01Z

I have fixed the requested changes. Could you review it again, please? @guilherme-gm

MrKeiKun · 2025-03-17T06:10:56Z

Squash and reword some commits within the PR. Some commits has messages that does not reflect the changes within it or just does not make any sense at all.

NPC_COMET Missing colon SkillInfo

Update Mob DB Contents Update Mob Contents Update mob database contents

Revert "Isle of Bios Instance" This reverts commit eed2b30. Instance Isle Of Bios cleanup description instance Improve styling and add instance re-entry Update IsleOfBios.txt Added variable for the next instance

Update PrizeOfHero Update Items missing

Vaans · 2025-03-20T15:18:37Z

Done, I guess... Sorry! Still figuring GitHub out. 😅

npc/re/instances/IsleOfBios.txt

guilherme-gm · 2025-04-05T14:38:25Z

npc/re/instances/IsleOfBios.txt

+	switch( questprogress(15005,PLAYTIME) ) {
+	case 0:
+		if(select("Create Instance", "Cancel") == 0 ) { close();	}
+			if( questprogress(15008,PLAYTIME) == 2) {


the indent of the entire case block seems wrong, it has 2 tabs instead of 1 (it should be aligned like the if in line 49, not with an extra indent) -- supposing that close above is correct.

Shouldn't the case also be indented? The case is nested inside the switch hence it's missing a tab. Which would however also mean the content inside the if is missing tabs. But unfortunately according to the coding style it is correct.

At least looking into the script changes from the last 2 years, and at a quick search through the npc folder, case should not be indentended, just like our style guide says (but there are some places where it is not -- probably left overs/something that passed a review)

Yes, our coding style says:

switch () { case 1: <tab>[code]

which is the pattern we've been adopting everywhere as far as I know.

I would be ok if the script is using

switch () { <tab>case 1: <tab><tab>[code]

too, as long as it is consistent across the entire script, but I think it is better to adhere to our style guide (case not indented)

guilherme-gm · 2025-04-05T14:39:44Z

npc/re/instances/IsleOfBios.txt

+				mes("^0000ffThis Memorial Dungeon cannot be accessed for 23 hours after your last visit.^000000");
+				close();
+			}
+			if (getcharid(CHAR_ID_CHAR) != getpartyleader(.@party_id, 2)) {


party id is not set here.

guilherme-gm · 2025-04-05T14:46:45Z

npc/re/instances/IsleOfBios.txt

+				mes("^ff0000Created the Memorial dungeon.^000000");
+				mes("^ff0000Please click and press Enter again.^000000");
+				close;
+				}


indent issue

I'm confused about the inconsistent close; usage. Though I haven't coded and kept up but where is there close(); and close;?

both produces the same result. close(); is ideal as it matches the "updated standard", while close; will be matching the old standard, which is still supported just fine.

While I do think using close(); everywhere is preferable, I am not enforcing this in my review since it is a conversion from rA (which as far as I know doesn't use the () around functions unless needed).

It would be nice to have it in the new standard already, or at least have everything either in the new or old standard instead of a mix. But I do understand the extra effort in making this happen, and since most of our scripts are not in the new format yet, I would say it is ok to use the old one.

If I recall correctly, except for end;, every other script command should be using ()

Got it.
Yeah I only found one reference of end(); in https://github.com/HerculesWS/hercules-docs/blob/19e89784901aa6e8a53d98f44cfecd44460601d5/docs/scripting/timers.md#use-number-3-deny-usage

guilherme-gm · 2025-04-05T14:48:48Z

npc/re/instances/IsleOfBios.txt

+				mes("^ff0000Please click and press Enter again.^000000");
+				close;
+				}
+	case 1:


is the fall through from case 0 to 1 intended? if .@instance is < 0 it will fall through and run case 1. this seems wrong to me

npc/re/instances/IsleOfBios.txt

guilherme-gm · 2025-05-16T11:47:24Z

npc/re/instances/IsleOfBios.txt

+	switch( questprogress(15005,PLAYTIME) ) {
+	case 0:
+		if(select("Create Instance", "Cancel") == 0 ) { close();	}
+			if( questprogress(15008,PLAYTIME) == 2) {


guilherme-gm · 2025-05-16T11:48:41Z

npc/re/instances/IsleOfBios.txt

+	}
+	switch( questprogress(15005,PLAYTIME) ) {
+	case 0:
+		if(select("Create Instance", "Cancel") == 2 ) {


please fix the spacing to align with Hercules style guide - https://github.com/HerculesWS/Hercules/wiki/Coding-Style

guilherme-gm · 2025-05-16T11:51:17Z

npc/re/instances/IsleOfBios.txt

+				mes("^ff0000Created the Memorial dungeon.^000000");
+				mes("^ff0000Please click and press Enter again.^000000");
+				close;
+				}


guilherme-gm · 2025-05-16T11:52:36Z

npc/re/instances/IsleOfBios.txt

+				mes("^ff0000Please click and press Enter again.^000000");
+				close;
+				}
+	case 1:


Vaans changed the title ~~Initiale release of Isle of Bios~~ Initia release of Isle of Bios Feb 11, 2025

Vaans changed the title ~~Initia release of Isle of Bios~~ Initial release of Isle of Bios Feb 11, 2025

guilherme-gm requested changes Mar 13, 2025

View reviewed changes

Vaans requested a review from guilherme-gm March 13, 2025 23:27

Vaans force-pushed the isle-of-bios branch from ce080ca to cc1d04d Compare March 17, 2025 13:00

NPC_COMET

ecc5316

NPC_COMET Missing colon SkillInfo

Vaans force-pushed the isle-of-bios branch 2 times, most recently from 7859788 to 7de0fff Compare March 17, 2025 14:08

Vaans added 3 commits March 20, 2025 12:10

Quest & Mob Contents

a64c8f7

Update Mob DB Contents Update Mob Contents Update mob database contents

Isle of Bios Instance

b206360

Revert "Isle of Bios Instance" This reverts commit eed2b30. Instance Isle Of Bios cleanup description instance Improve styling and add instance re-entry Update IsleOfBios.txt Added variable for the next instance

Item_db update

42fd874

Update PrizeOfHero Update Items missing

Vaans force-pushed the isle-of-bios branch 2 times, most recently from 4440050 to 42fd874 Compare March 20, 2025 15:13

guilherme-gm requested changes Apr 5, 2025

View reviewed changes

Update IsleOfBios.txt

73e48b0

Vaans requested a review from guilherme-gm May 8, 2025 02:46

guilherme-gm requested changes May 16, 2025

View reviewed changes

Initial release of Isle of Bios #3337

Are you sure you want to change the base?

Initial release of Isle of Bios #3337

Uh oh!

Conversation

Vaans commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guilherme-gm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Vaans commented Mar 13, 2025

Uh oh!

MrKeiKun commented Mar 17, 2025

Uh oh!

Vaans commented Mar 20, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Vaans commented Feb 11, 2025 •

edited

Loading