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

feat: Epitaph Road #619

Merged
merged 6 commits into from
Nov 26, 2024
Merged

Conversation

pacampbell
Copy link
Collaborator

@pacampbell pacampbell commented Oct 27, 2024

Added support for Epitaph Road endgame content.

  • Implemented the personal quest to enter "Heroic Spirit Sleeping Path:
    Rathnite Foothills".
  • Created new epitaph assets which describe the different paths,
    sections and trials.
  • Implemented multiple packet handlers for different season dungeon
    packets.
  • Implemented queued packet handler which enables packets to be send
    after the response is sent.
  • Implemented sections 1, 2, 3 and 4 for Rathnite Foothills
  • Implemented the final trial for Rathnite Foothills
  • Implemented sections 1 and 3 for Memory of Megadosys.

image

Checklist:

  • The project compiles
  • The PR targets develop branch

@pacampbell pacampbell force-pushed the epitaph_road branch 11 times, most recently from 2756a27 to db4c6e4 Compare November 3, 2024 02:44
@pacampbell pacampbell force-pushed the epitaph_road branch 7 times, most recently from ad2777a to 6c85168 Compare November 11, 2024 01:09
@pacampbell pacampbell force-pushed the epitaph_road branch 12 times, most recently from 687996c to 3f2e4af Compare November 18, 2024 15:05
@pacampbell pacampbell force-pushed the epitaph_road branch 11 times, most recently from 314aabc to 7071eaf Compare November 24, 2024 23:35
- Implemented many req, res, ntc and handlers for Epitaph content.
- Renamed existing BonusDungeonManager to DungeonManager.
- Implemented database migration from 24 -> 25.
- Implemented a new packet handler which provides a queue which can be
  used to send packets after the response is returned.
- Commented out RPC ping print message.
- Added new NPC behavior to NpcGetExtendedFacilityHandler.
- Created personal quest required to enter Rathnite Foothills dungeon.
- Created new EpitaphRoad.json file which contains dungeon information
  for each path.
- Created new epitaph directory to hold all trials
  - Implemented all trials for Rathnite foothills.
  - Implemented section 1 and section 3 trials for Memory of Megadosys.
- Added docs which contain information found about buffs, objectives and rewards.
- Started a new document guide.md which describes the epitaph assets.
- Added new enemies for the Rathnite Foothills dungeon.
- Added some enemies to memory of megadosys.
@pacampbell pacampbell marked this pull request as ready for review November 25, 2024 00:07
@@ -59,6 +59,14 @@ public Character SelectCharacter(uint characterId, DbConnection? connectionIn =
return null;
}

character.EpitaphRoadState.UnlockedContent = _Server.Database.GetEpitaphRoadUnlocks(character.CharacterId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

At some point we need to move a lot of this into a transaction and/or make this more modular. Fetching a character shouldn't involve a dozen DB opens and closes, and not every character fetch needs every detail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure what you mean by this. Every character needs the details populated. Old logic should be moved into the character manager instead of the way it was originally designed so we can move the transaction logic into these functions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you attempt to check the profile of characters on other channels? I can't remember. If you can, you definitely need some of their details but you certainly don't need their Epitaph road state.

See also the current implementation in PawnRentRegisteredPawnHandler, where we have to do a full fetch of the owner's details just to get their inventory.

Copy link
Collaborator Author

@pacampbell pacampbell Nov 25, 2024

Choose a reason for hiding this comment

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

This function is called when you log in. It manages collecting and calculating stats from BO trees, pawns and all the other auxiliary information. It should only be called once (or maybe twice because we query twice at login for some reason).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see it got used in some of these pawn ones. Maybe we should make a lighter ones for those? Problem is pawns need the character inventory and all that other unpleasant stuff.

@@ -21,7 +21,7 @@ public RpcServer(DdonGameServer gameServer)
public RpcCommandResult Execute(IRpcCommand command)
{
RpcCommandResult result = command.Execute(_gameServer);
Logger.Info(result.ToString());
// Logger.Info(result.ToString());
Copy link
Collaborator

Choose a reason for hiding this comment

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

As I continue to add RPC stuff, I do think we need to have logging for it. Maybe in a better form than it currently is, but it should be there in some form.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This log spams so often, it gets in the way of reading the logs when ddon-tools is connected. We should make some filter system for this so we only get logs we want.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not against removing this comment. It's true it gets spammy and i don't think we're getting much info out of it

@pacampbell pacampbell force-pushed the epitaph_road branch 3 times, most recently from 6f60612 to 2aa0ce4 Compare November 25, 2024 05:55
- Add cancel handler
- Fix bug when party ready/unready logic
- Add missing connectionIn to DB queries
- Fix argument id in om command
- Fix issue where repop NTC was only sent to the player who killed the
  enemy.
- Changed certain OM update packets to only send to the client who
  requested.
- Fix barrier purchase so it only unlocks for the person who bought.
Copy link
Collaborator

@alborrajo alborrajo left a comment

Choose a reason for hiding this comment

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

These features are getting crazy complicated, good job on it.
I haven't focused too much on the packets, managers, and handlers as i trust that they work, the comments im giving are mostly nitpicks, places where i feel are missing comments or explaining where certain values come from, and suggestions that you can freely ignore if you wish.

CONSTRAINT "fk_ddon_epitaph_road_unlocks_character_id" FOREIGN KEY ("character_id") REFERENCES "ddon_character"("character_id") ON DELETE CASCADE
);

CREATE TABLE IF NOT EXISTS ddon_epitaph_claimed_weekly_rewards (
Copy link
Collaborator

Choose a reason for hiding this comment

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

As an idea you could evaluate, rather than periodically cleaning up this table, you could store a timestamp and make sure there's no entry from the current week

// result as seen in player videos
results.Add(new InstancedGatheringItem()
{
ItemId = (gatheringPoint == null) ? 9393 : dungeonInfo.SoulItemId,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these magic numbers be configurable? Or at least be named constants

Copy link
Collaborator Author

@pacampbell pacampbell Nov 26, 2024

Choose a reason for hiding this comment

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

I suppose this could be more robust than it currently is. Will add a TODO for now to make it more configurable.

return 0;
}

private readonly List<(EpitaphBuffId BuffId, uint ItemId, uint Amount, double Chance)> gDropConfigs = new List<(EpitaphBuffId BuffId, uint ItemId, uint Amount, double Chance)>()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as before, should these be configurable? If they should but it's not done yet please leave a TODO. If they shouldn't be configurable a named constant or a comment explaining would be useful

Copy link
Collaborator Author

@pacampbell pacampbell Nov 26, 2024

Choose a reason for hiding this comment

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

They could be configurable, will add a TODO. They are used for adding bonus items from the epitaph buffs.

- Added missing object instantiations to packet consturctors.
- Renamed packets which were not named correctly.
- Added some TODO comments
@pacampbell pacampbell requested a review from alborrajo November 26, 2024 14:19
@pacampbell pacampbell merged commit 2b148dc into sebastian-heinz:develop Nov 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants