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.

Sign up for GitHub

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

Jump to bottom

Fix EmpyreanCrucibleInstance #5

Open
wants to merge 10 commits into
base: 4.8
Choose a base branch
from
Open

Fix EmpyreanCrucibleInstance #5

wants to merge 10 commits into from

Conversation

w4terbomb
Copy link
Contributor

@w4terbomb w4terbomb commented Jun 27, 2024 •
edited
Loading

I tried to do the best I could with the time I had, I wasn't able to test as much as I would have liked. I believe that adjustments and even npcAI may be missing. I need more information to be able to do a better job, if someone could test and point out the errors I could correct them all.

Changes:

  • Fix spawns
  • Fix time (rounds)
  • Added new stages and rounds
  • Refactored all code and logic
  • Added Kaisinel(Elyos) and Marchutan(Asmodians) [Retail]
  • Added random stages [Retail]
  • Added some NPCs that were missing.

(https://www.youtube.com/watch?v=dDTlv5iKlr4&list=PL7519772E07E8BE68&index=1)

@Estrayl
Copy link
Member

Estrayl commented Jul 1, 2024

Hey, I finally found the time to review your changes (couldn't test them though).

There are quite a few occurrences of SkillEngine.getInstance().useSkill(), which has some drawbacks. This should not be used for NPCs that can be CCed, as they will not be able to execute the skill properly. Also, this will interrupt any skill that is currently being cast.
A better approach is the skill queue we implemented, which can be accessed by calling getOwner().queueSkill(), so the NPC will only cast the skill when he is ready.
Except for a few cases, it is also completely possible to move any skill execution into the npc_skills templates, using min_hp, max_hp, min_time, max_time, chain_id, and conjuction_type.

If you need more information about the instance and the AIs, you would have to extract it from the 4.6 PTS leak.
Spawns and their conditions can be found in ./World/IDArena/world.xml. If you want to go the extra mile, you can also analyze the retail AIs (the leak with AI samples should contain one or more files with IDArena_). This may take some time to fully understand and replicate though.

Ofc, you can contact me on discord if you need help analyzing the retail data.

In summary, it would be nice to remove the SkillEngine calls and handle this with the SkillQueue instead.

neon-dev
neon-dev reviewed Aug 4, 2024
View reviewed changes
game-server/data/handlers/instance/crucible/EmpyreanCrucibleInstance.java
npc.getController().delete();
break;
case 217502:
sendMsg(SM_SYSTEM_MESSAGE.STR_MSG_INSTANCE_COMPLETE_ROUND_IDARENA());
Copy link
Member

Choose a reason for hiding this comment

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

This does not compile due to the missing parameter. It also looks like this message should be used in other places too, as well as STR_MSG_INSTANCE_START_ROUND_IDARENA and STR_MSG_INSTANCE_JOIN_ROUND_IDARENA.

neon-dev
neon-dev reviewed Aug 4, 2024
View reviewed changes
game-server/data/handlers/ai/instance/empyreanCrucible/VanktristAI.java
@Override
public void handleHpPhase(int phaseHpPercent) {
switch (phaseHpPercent) {
case 100 -> PacketSendUtility.broadcastMessage(getOwner(), 0); // FIXME
Copy link
Member

Choose a reason for hiding this comment

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

The message ID is missing here.

neon-dev
neon-dev reviewed Aug 5, 2024
View reviewed changes
game-server/data/handlers/ai/instance/empyreanCrucible/VanktristAI.java
Comment on lines +46 to +49
PacketSendUtility.broadcastMessage(getOwner(), 1500210); // STR_CHAT_IDArena_STAGE7_D_R1_Die TODO correct msg
startPhaseTask();
}
case 50 -> PacketSendUtility.broadcastMessage(getOwner(), 1500211); // STR_CHAT_IDArena_STAGE7_L_R2_Skill1 TODO different msg for L/D
Copy link
Member

Choose a reason for hiding this comment

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

The message IDs are not correct.
Example: STR_CHAT_IDArena_STAGE7_D_R1_Die

  • D means dark faction (Asmodians). Each faction has their own message (Elyos have L)
  • R1 means round 1, R2 round 2, and so on. Each round has their own messages.
  • Die means the message belongs to the death event of the npc.
    There is also Skill1, Skill2 and so on, which corresponds to a specific skill cast event. Not sure if messages are tied to certain skill IDs or if it's a random chance on each skill cast.

neon-dev
neon-dev requested changes Aug 5, 2024
View reviewed changes
game-server/data/handlers/ai/instance/empyreanCrucible/PriestPreceptorAI.java Outdated
public void handleBackHome() {
super.handleBackHome();
hpPhases.reset();
ThreadPoolManager.getInstance().schedule(() -> getOwner().queueSkill(19612, 46), 1000);
Copy link
Member

Choose a reason for hiding this comment

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

All skill casts in this file have changed to skill level 46. Is this correct?

game-server/data/handlers/ai/instance/empyreanCrucible/RM1337AI.java
@Override
public void run() {
if (!isDead()) {
SkillEngine.getInstance().getSkill(getOwner(), 19553, 10, target).useNoAnimationSkill();
Copy link
Member

Choose a reason for hiding this comment

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

19553 no longer gets cast in this AI. Was this an intentional change?

Copy link
Member

Choose a reason for hiding this comment

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

There is also 19554, which could belong to this npc (haven't checked).

game-server/data/static_data/npcs/npc_templates.xml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Are all marabata and marabata_controller changes related to this instance? Because the handlers are used in Dark Poeta.
The marabata AI even spawns other npcs at Dark Poeta coordinates, so you at least need to modify those AIs or extend / copy them.

game-server/data/handlers/ai/instance/empyreanCrucible/SparkOfDarknessAI.java
@@ -28,11 +27,16 @@ protected void handleSpawned() {
private void startEventTask() {
ThreadPoolManager.getInstance().schedule(() -> {
if (!isDead())
SkillEngine.getInstance().getSkill(getOwner(), 19554, 1, getOwner()).useNoAnimationSkill();
getOwner().queueSkill(19554, 1);
Copy link
Member

Choose a reason for hiding this comment

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

Queueing a skill on spawn does not cast it immediately, and only if the npc has aggro towards a target in range.
Therefore skills cast on spawn should not use queueSkill.

neon-dev
neon-dev reviewed Aug 5, 2024
View reviewed changes
game-server/data/handlers/ai/instance/empyreanCrucible/KingConsierdAI.java
*/
@AIName("king_consierd")
public class KingConsierdAI extends AggressiveNpcAI implements HpPhases.PhaseHandler {

private final HpPhases hpPhases = new HpPhases(75, 25);
private AtomicBoolean isHome = new AtomicBoolean(true);
private final AtomicBoolean isHome = new AtomicBoolean(true);
Copy link
Member

Choose a reason for hiding this comment

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

This field can be removed by instantiating hpPhases via new HpPhases(100, 75, 25) and starting the tasks from handleHpPhase in case 100.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neon-dev neon-dev neon-dev requested changes

@Estrayl Estrayl Estrayl left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@w4terbomb @Estrayl @neon-dev