Skip to content
This repository has been archived by the owner on Sep 16, 2024. It is now read-only.

compatibility changes for dadbattle + philly #789

Merged
merged 11 commits into from
Jun 10, 2021

Conversation

Lucky-56
Copy link

@Lucky-56 Lucky-56 commented Jun 8, 2021

fixes:

  • 'Dad Battle' now has compatibility again yay

'Dad Battle' is now called 'Dad Battle' in game and uses 'dadbattle' as files
'Philly' is now called 'Philly Nice' in game and uses 'philly' as files (reverted #760 and instead fixed it better)

'Dad Battle' is now called 'Dad Battle' in game and uses 'dadbattle' as files
'Philly' is now called 'Philly Nice' in game and uses 'philly' as files
@Lucky-56
Copy link
Author

Lucky-56 commented Jun 8, 2021

@theDetourist here ya go

@@ -202,13 +202,24 @@ class FreeplayState extends MusicBeatState

if (accepted)
{
trace(StringTools.replace(songs[curSelected].songName," ", "-").toLowerCase());
// pre lowercasing the song name
var songLowercase = switch (songs[curSelected].songName)
Copy link
Collaborator

@puyoxyz puyoxyz Jun 8, 2021

Choose a reason for hiding this comment

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

This switch case is hella wack you're repeating code and changing like one argument

Do something like this instead

var songLowercase = StringTools.replace(PlayState.SONG.song, " ", "-").toLowerCase();
switch (songLowercase) {
  case 'dad-battle': songLowercase = 'dadbattle';
  case 'philly-nice': songLowercase = 'philly';
}

source/Paths.hx Outdated
@@ -97,14 +97,30 @@ class Paths

inline static public function voices(song:String)
{
song = StringTools.replace(song," ", "-");
return 'songs:assets/songs/${song.toLowerCase()}/Voices.$SOUND_EXT';
var songLowercase = switch (song)
Copy link
Collaborator

Choose a reason for hiding this comment

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

See previous

source/Paths.hx Outdated
}

inline static public function inst(song:String)
{
song = StringTools.replace(song," ", "-");
return 'songs:assets/songs/${song.toLowerCase()}/Inst.$SOUND_EXT';
var songLowercase = switch (song)
Copy link
Collaborator

Choose a reason for hiding this comment

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

see previous

source/PlayState.hx Show resolved Hide resolved
source/PlayState.hx Show resolved Hide resolved
@@ -1368,9 +1389,20 @@ class PlayState extends MusicBeatState

var playerCounter:Int = 0;

// pre lowercasing the song name (generateSong)
var songLowercase = switch (PlayState.SONG.song)
Copy link
Collaborator

Choose a reason for hiding this comment

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

see previous

source/PlayState.hx Show resolved Hide resolved

if (SONG.song.toLowerCase() == 'eggnog')
// pre lowercasing the song name (endSong)
var songLowercase = switch (PlayState.SONG.song)
Copy link
Collaborator

Choose a reason for hiding this comment

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

see previous

source/Song.hx Outdated
var rawJson = Assets.getText(Paths.json(StringTools.replace(folder," ", "-").toLowerCase() + '/' + StringTools.replace(jsonInput," ", "-").toLowerCase())).trim();
trace(jsonInput);
// pre lowercasing the folder name
var folderLowercase = switch (folder)
Copy link
Collaborator

Choose a reason for hiding this comment

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

see previous

@Lucky-56
Copy link
Author

Lucky-56 commented Jun 8, 2021

sure, changes comin' right up

"This switch case is hella wack you're repeating code and changing like one argument"
@Lucky-56 Lucky-56 requested a review from puyoxyz June 8, 2021 11:20
Copy link
Collaborator

@puyoxyz puyoxyz left a comment

Choose a reason for hiding this comment

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

Looks good; @Lucky-56 did you test it with the new changes?

@Lucky-56
Copy link
Author

Lucky-56 commented Jun 8, 2021

ok... wierd things are happening.
I start Philly in freeplay, but I play Satin Panties... ?

@Lucky-56
Copy link
Author

Lucky-56 commented Jun 8, 2021

ah ok I broke Freeplay, nothing I can't fix

@Lucky-56
Copy link
Author

Lucky-56 commented Jun 8, 2021

it just crashes when going into freeplay now.. and I'm too tired to fix it now :/

highscore would be compatible if the package wasn't changed to KadeDev's own
@Lucky-56 Lucky-56 marked this pull request as ready for review June 8, 2021 22:37
@Lucky-56
Copy link
Author

Lucky-56 commented Jun 9, 2021

it works now tested it completely

Copy link
Collaborator

@puyoxyz puyoxyz left a comment

Choose a reason for hiding this comment

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

Yep

switch (songHighscore) {
case 'Dad-Battle': songHighscore = 'Dadbattle';
case 'Philly-Nice': songHighscore = 'Philly';
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

there has to be a less janky way to do this but this whole codebase is wack so whatever

@puyoxyz puyoxyz requested a review from Kade-github June 9, 2021 10:29
@Lucky-56
Copy link
Author

Lucky-56 commented Jun 9, 2021

just checked and remembered

  1. it doesn't even get called at the beginning of the song
  2. it's only a problem on my mod 😅

Copy link

@isakube isakube left a comment

Choose a reason for hiding this comment

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

imagine making a commit and then immediately reverting it LOLL

@Lucky-56
Copy link
Author

Lucky-56 commented Jun 9, 2021

yeah because I noticed it right when I commited... heh

@Lucky-56
Copy link
Author

Lucky-56 commented Jun 9, 2021

Fixes #779
#790 don't know yet would assume it fixes that
Fixes #796
Fixes #804
Fixes #805
i guess

Copy link

@ActualMandM ActualMandM left a comment

Choose a reason for hiding this comment

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

Satin Panties and Winter Horrorland also need compatibility changes.

Copy link

@ActualMandM ActualMandM left a comment

Choose a reason for hiding this comment

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

nvm, disregard what i said

@puyoxyz
Copy link
Collaborator

puyoxyz commented Jun 10, 2021

@kadedev ??????????????????

@Kade-github Kade-github merged commit c649932 into KadeArchive:master Jun 10, 2021
@Kade-github
Copy link
Collaborator

sorryy!!!

@Lucky-56
Copy link
Author

yeah it's only hardcoded because it's for compatibility

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants