Implemented die roll animations for the Lego Dice item

[ckawell]

The Lego Dice item does not complete its roll animation (just spins in the air before disappearing). This is because the client lua script is not implemented server-side. This PR adds the server-side c++ script and header.

IMPORTANT: I was not able to test this with multiple accounts, so I'm not sure if this fixes the animations from the view of other players on the server. This should ideally be tested before merging.

[NealSpellman]

Were you planning on including support for the achievement that requires rolling the dice? Ideally that'd be included in this same PR, imo.

[jgkawell]

IMPORTANT: I was not able to test this with multiple accounts, so I'm not sure if this fixes the animations from the view of other players on the server. This should ideally be tested before merging.

Let's test this out later today and report back.

[codeshaunted]

IMPORTANT: I was not able to test this with multiple accounts, so I'm not sure if this fixes the animations from the view of other players on the server. This should ideally be tested before merging.

Let's test this out later today and report back.

If I recall correctly, the GameMessages::SendPlayAnimation function broadcasts by default, so I'd assume that everyone can see the animation.

[ckawell]

Were you planning on including support for the achievement that requires rolling the dice? Ideally that'd be included in this same PR, imo.

Can't figure out how to, tbh. To do that I'd need a reference to the current player which I'm having trouble getting. If someone who has more experience with this codebase than one weekend wants to add a commit including the achievement, that'd be great.

[ckawell]

IMPORTANT: I was not able to test this with multiple accounts, so I'm not sure if this fixes the animations from the view of other players on the server. This should ideally be tested before merging.

Let's test this out later today and report back.

Sounds good, I'm down.

[cooltrain7]

I think it would be a good idea to start using logger::LogDebug for new logs going forward unless it needs to be seen by the server host. As it stands there is so much verbose logging that is only needed during debugging. Thoughts?

[ckawell]

I think it would be a good idea to start using logger::LogDebug for new logs going forward unless it needs to be seen by the server host. As it stands there is so much verbose logging that is only needed during debugging. Thoughts?

Good idea, I totally agree. Wouldn't hurt to go back and clean up a bunch of those in a future PR.

[Nordegraf]

Were you planning on including support for the achievement that requires rolling the dice? Ideally that'd be included in this same PR, imo.

Can't figure out how to, tbh. To do that I'd need a reference to the current player which I'm having trouble getting. If someone who has more experience with this codebase than one weekend wants to add a commit including the achievement, that'd be great.

I added tracking of the achievement in the same way i added the tracking of the Pet Excavator achievement in PR #213. You can fetch the player object by calling GetOwner on the self entity and then forcing progress on the mission. I opened a PR on your repository, if you want to have a look.

[ckawell]

@Nordegraf Awesome, thanks. Good to know for future reference. I'll test it out and merge it into this PR later tonight hopefully.

[ckawell]

Tested with a colleague and looks good. The animations are rendering for both players and the achievement is updating on 6 rolls.

[Xiphoseer]

Note that the diff for this PR includes a new submodule utils in the root folder which GitHub can't resolve. I assume that's lcdr's utils and unintentional.

[ckawell]

@Jettford I personally don't feel strongly either way (this is a very small PR with simple, short logic), but I will say that a six case switch statement isn't even remotely large; it's what they exist for.
But developers have different programming styles and it's your codebase after all, so feel free to change it however you wish. I really don't mind others' contributions to this PR. Thanks

[ckawell]

The sooner this fix can be merged the better though. This is a real bug in the game that people are spending 15,000 coins to experience. I write national healthcare software that affects just about every clinician in the country, and we have far less strict coding conventions. The most important thing is to get functional, reasonable fixes for bugs in production as soon as possible.
I'm sorry if this offends you. I'm not trying to be rude, I just want to keep things in perspective.

[ckawell]

@Wincent01 any chance we can get this expedited? I think this has taken up enough folks' valuable time.

[Wincent01]

Looks good, merging. This codebase is a lost cause for perfect code, this implementation is fine. It's unlikely anyone will have to touch this script again, so why put this much effort into making it look good.