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

New judge application logic #2328

Merged
merged 5 commits into from
May 30, 2024

Conversation

nebulazorua
Copy link
Contributor

@nebulazorua nebulazorua commented May 8, 2024

Does a couple of things:

  • Adds hitDiff to HitNoteScriptEvent
  • Adds isComboBreak to HitNoteScriptEvent
  • Adds doesNotesplash to HitNoteScriptEvent
  • Moved code related to applying the judgement stats (score, health, etc) to a new applyScore function
  • popupScore now only has 2 arguments, judgement and combo.

So, why these changes?
popupScore was a bit of a misleading title, as it doesn't just display the judges, but it also applied logic related to them, so I have split the functionality between popupScore and applyScore. This, in my opinion, makes a lot more sense and allows for scripts to display judgements seperately from score application, i.e in the case of mine/hurt notes where maybe they'd want to display a custom judgement to show that you hit a mine. Perhaps HitNoteScriptEvent.showScore can be a thing, which'd toggle whether popupScore is called or not (though in scripts you could just event.cancel and call applyScore yourself without calling popupScore to hide the judgement display)

The changes to HitNoteScriptEvent to add doesNotesplash , hitDiff and isComboBreak is to give more freedom to modders when doing things on note hit, such as special note kinds with lower hitboxes, like mines, or even custom judgements.

Mines, when implemented in FNF mods, generally combo break and have a smaller hitbox, so an example Mine script's onNoteHit could look a bit like this:

override function onNoteHit(event){
  if(event.note.kind == 'mine'){
    var noteDiff = event.hitDiff;
    if(Math.abs(noteDiff) > 60){ // Lowers the hitbox from the full 166ms to 60ms.
      event.cancel();
      return;
    }
    event.isComboBreak = true;
    event.healthChange = -0.2;
    event.judgement = 'mine';
    event.score = -150;
    event.doesNotesplash = false;
  }
}

This would not be as easily possible without these changes to HitNoteScriptEvent and how score is applied.

Also adds more to the HitNoteScriptEvent, such as the noteDiff and also isComboBreak.
noteDiff is important as it lets you create stuff like notes with reduced hitboxes (i.e mines/"hurt notes") or millisecond hit displays. isComboBreak lets you set whether the judge combo broke, so you could in theory create custom judgements in scripts now with these changes (killer reimplementation in scripts? more likely than you'd think!)
@crowplexus
Copy link

i was gonna do this myself so I could make the killer judge work but here we go

@nebulazorua nebulazorua changed the base branch from main to develop May 8, 2024 20:56
@EliteMasterEric
Copy link
Member

The diffs for this looked good, I'm going to have to test this in-game but otherwise this seems like a good set of features.

@EliteMasterEric EliteMasterEric self-requested a review May 9, 2024 05:49
@nebulazorua
Copy link
Contributor Author

Any update on if/when this might get merged? I've been thinking of updating it to add NoteHitScriptEvent.isSplash so you can turn on/off the note-splash. Would allow scripts to add custom judgements, or force certain note types to always splash for i.e explosions for mines, etc.

@nebulazorua
Copy link
Contributor Author

Updated the PR to include doesNotesplash in the list of changes i've made

@EliteMasterEric EliteMasterEric merged commit 2380058 into FunkinCrew:develop May 30, 2024
@nebulazorua
Copy link
Contributor Author

(ignore my last reply that i removed i was on the wrong branch teehee)

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.

None yet

3 participants