ProjectSWGCore / NGECore2

The ProjectSWG Open Source Core
projectswg.com
GNU Lesser General Public License v3.0
23 stars 70 forks source link

Added player music functionality #1273

Closed wallaceg09 closed 9 years ago

wallaceg09 commented 9 years ago

Refactored how the HashMap "performances" works in EntertainmentService.java to have a key consisting of the name and the audio id of the performance. I did this because those two values together uniquely identify each individual performance.

I also added the command "startmusic". The command should start the correct song (I tested with a few songs, and they all seemed to match up properly), it should correctly animate the player for most instruments (when I tested mandoviol the character didn't animate during the song intro), and it should properly send most music-related messages to the player.

/stopmusic is now implemented. I also refactored some of the "performance" functionality into the EntertainmentService.

Known bugs: -It seems that the intro animation for mandoviol doesn't work. -Calling the "startmusic" command with no arguments properly presents an SUI window with a choice of songs that the player can play with the current instrument, however it doesn't appear that the list is complete. Also, I'm not sure how the list should be sorted, so currently it is not sorted at all. -The "stopmusic" command is not implemented yet. [Resolved] -Switching instruments while playing simply switches the instrument, this is incorrect behavior. -Calling the "startmusic" command while already playing causes the new song to be played (I know how to fix this, I just haven't yet) -Flourishes haven't been implemented for music yet

[Fixed some comment typos] [Updated to reflect changes]

lewismorgan commented 9 years ago

Code looks pretty good from looking at it in the changes. One thing I should suggest is to not be afraid to create methods in the entertainment service to call with a script. The code ends up being much cleaner and easier to follow. Example, junk dealers were all done via script but it was hard to follow and debug so I created methods in loot service which did the complex logic. Also you may want to revisit dancing when you're done with music. Right now helper methods are being used in the object class which should be moved to either a service or script.

wallaceg09 commented 9 years ago

Yeah, I saw that with the deprecated methods in CreatureObject. I just finished refactoring Ent Service to handle stopping performances instead of having creature object do it. I figured it would be best to slowly phase those deprecated methods out instead of trying to brute force them.

I do think I'm going to move some of the script code over to the Ent service for music because it would be easier to handle some of the logic in Java. After I get music working well enough I'll then start looking at dance to clean it up a bit.

Also I have a question about a parameter. In dance, the startPerformance method is called with this signature: "entSvc.startPerformance(actor, dance.getLineNumber(), -842249156 , danceVisual, 1)"

What is the large negative number for? I've just left the number the same for startMusic without understanding what it's for. It doesn't appear to have messed anything up catastrophically. The parameter is called "performanceCounter" in the Java code, but I haven't really been able to figure out what that counter is actually used for.

lewismorgan commented 9 years ago

That large number represents a integer that's sent in the delta for setting the performance from what I can recall (Been a couple months, I can't remember which one to be exact, you can trace the method to find it). I'm unsure if it's the same for music, you'd have to look at some music packets. Also, it could be that's the integer in the dancing performance datatables if not the packet. Dark originally setup dancing and I finished it up when he had to leave.