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

Implement SecNum for monster stats damage/HP #110

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Implement SecNum for monster stats damage/HP #110

wants to merge 9 commits into from

Conversation

AMXBus
Copy link
Contributor

@AMXBus AMXBus commented Dec 10, 2024

Another SecNum change but these are a bit more complex.

Main things to note:

  • We implement SecNum calls in the runtime intit structure just like before for the monster damage/HP stats (currently in CREATURELOCKER.as and RebalancedCreatures.as if we ever want to use those)
  • The calls are a little bit different here as we incorporate the champion monsters as well (these stats are in CHAMPIONCAGE.as)
  • This is the first moment where I have seen pointers (primitive but it's something) being used in this game as CHAMPIONCAGE.as and CREATURELOCKER.as have their own getter functions for properties. (CHAMPIONCAGE.GetGuardianProperty() and CREATURES.GetProperty())
  • All POPUP equivalents use these property getters that are essentially full object passers so we need to be careful if we plan any enhancements further down the line.
  • Some buildings use these monster properties for their own functions like bunkers and champion cage healing (housing as well if MR3 is active)
  • There is a lot of complex math that gets involved at every tick() but is well secured with prior SecNum implementation
  • This should not interfere with how the game calculates values for buffs/armor/dmg reduction as all that math is well in the background and works on each tick(). If any changes would happen mid calculation it would either crash the game or correct itself on the next one as these values get recalculated... all the time...

@AMXBus
Copy link
Contributor Author

AMXBus commented Dec 10, 2024

These changes have also been implemented for the inferno monsters (forgot tomention)

@React1-X React1-X added beta Beta related issue or feature client Client related issues enhancement New feature or request labels Dec 11, 2024
React1-X
React1-X reviewed Dec 12, 2024
View reviewed changes
client/scripts/CHAMPIONCAGE.as
@@ -480,6 +480,11 @@ package
{
var _loc2_:Object = _guardians["G" + param1];
var _loc3_:Object = _loc2_.props;
for (var temp = 0; temp < _loc3_.health.length; temp++)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @AMXBus - would you mind explaining the reason for introducing this loop here? I noticed it's also called "temp", is this accidentally included in the PR?

client/scripts/CREATURES.as
@@ -31,7 +31,7 @@ package
_guardianList.length = 0;
}

public static function GetProperty(param1:String, param2:String, param3:int = 0, param4:Boolean = true) : Number
public static function GetProperty(param1:String, param2:String, param3:int = 0, param4:Boolean = true) : *
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to do this? Should we not always expect a number returned from this function? Rather than anything? Or is there a good reason why we did this?

client/scripts/CREATURELOCKER.as
@@ -1131,7 +1132,20 @@ package
{
var _loc2_:Object = _creatures[param1];
var _loc3_:Object = _loc2_.props;
tmpArray.push([_loc2_.page,_loc2_.resource,_loc2_.time,_loc2_.level,_loc2_.trainingCosts,_loc3_.speed,_loc3_.health,_loc3_.damage,_loc3_.armor,_loc3_.accuracy,_loc3_.cTime,_loc3_.cResource,_loc3_.cStorage,_loc3_.bucket,_loc3_.size]);
for (var temp = 0; temp < _loc2_.trainingCosts.length; temp++)
Copy link
Contributor

@React1-X React1-X Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Similar question here; what's the reasoning for introducing new code here? is it absolutely necessary for this to work? I see this one is also called "temp", if it's needed and there's a good reason for this, then we should definitely rename these vars.

I need a good insight into this, as I'm a little skeptical about introducing new code into the client unless absolutely necessary.

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

@React1-X React1-X React1-X left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@AMXBus AMXBus

Labels
beta Beta related issue or feature client Client related issues enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AMXBus @React1-X @marti-kips