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

Added support for a new PPDrop field in enemy information #546

Conversation

nbfields2
Copy link
Contributor

@nbfields2 nbfields2 commented Sep 9, 2024

I have created a new PPDrop field for enemies. It is used to determine the number of Play Points an enemy drops upon defeat. It is to be used as an alternative to the current "Experience / 7500" system. In the case that no number of Play Points is specified, the old system is defaulted to. The below image contains an example of two enemies dropping 456 and 123 PP respectively, which I used as test values for the system.

pp

Checklist:

  • The project compiles
  • The PR targets develop branch

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.

Mostly good, just a few comments. I assume you tried it with both files that contain the PPDrop values and files that don't and enemies dropped the expected values in both cases, right?

Arrowgene.Ddon.Shared/Files/Assets/EnemySpawn.json Outdated Show resolved Hide resolved
@@ -68,6 +67,7 @@ public Enemy(Enemy enemy)
public uint Experience { get; set; }
public DropsTable DropsTable { get; set; }
public bool NotifyStrongEnemy { get; set; }
public uint PPDrop {get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: a space between the { and get;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have resolved each of these. I have also explicitly tested it on a server with two enemies, which I had not done before. I set two goblins to have PP drops of 123 and 456. Please check the PR description and you will see an in-game screeenshot demonstrating the PP were dropped. I can also confirm that loading the server with the old JSON (no PPDrop parameter is contained therein) loaded in the expected manner and started the server successfully, though I didn't enter the game and check the PP being dropped.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the space seems to be missing still 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed - for real this time!

@nbfields2 nbfields2 force-pushed the 395-enemy-playpoint-drop-through-new-optional-field-enemyspawn-json branch from 5bb3021 to 733bf6e Compare September 9, 2024 17:45
@alborrajo alborrajo linked an issue Sep 9, 2024 that may be closed by this pull request
@nbfields2 nbfields2 force-pushed the 395-enemy-playpoint-drop-through-new-optional-field-enemyspawn-json branch 2 times, most recently from f6a432d to 6390077 Compare September 13, 2024 14:41
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this file work with the server as it is? Wouldn't it reject the file since the schema contains the PPDrop field but the rows don't?
Keep in mind the server starting isn't enough, make sure it starts and loads the file successfully, it's easy to check if you try finding the enemies in their locations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies - you are correct, it would not work as-is. I did my testing with a different Enemy.cs file that only contained two enemies for easier testing. I removed the extra PPDrop field item.

I can confirm that a file without PPDrop works, and a file with PPDrop and enemies with the correct field also works, in both cases giving the expected amount of Play Points (experience / 7500 or the defined amount respectively).

@@ -68,6 +67,7 @@ public Enemy(Enemy enemy)
public uint Experience { get; set; }
public DropsTable DropsTable { get; set; }
public bool NotifyStrongEnemy { get; set; }
public uint PPDrop {get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

the space seems to be missing still 😛

@nbfields2 nbfields2 force-pushed the 395-enemy-playpoint-drop-through-new-optional-field-enemyspawn-json branch 2 times, most recently from fdd022c to 7d3bfd9 Compare September 17, 2024 16:51
@nbfields2 nbfields2 force-pushed the 395-enemy-playpoint-drop-through-new-optional-field-enemyspawn-json branch from 7d3bfd9 to 7e8fe4e Compare September 17, 2024 16:53
@alborrajo alborrajo merged commit d4cf46d into sebastian-heinz:develop Sep 17, 2024
1 check passed
@alborrajo
Copy link
Collaborator

Thanks for your contribution!

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.

Enemy play point drop through a new optional field in EnemySpawn.json
2 participants