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

Replace HP/breath by generic stat mechanism #1335

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@sapier
Copy link
Contributor

sapier commented May 25, 2014

This is not compatible to current servers and requires a min protocol bump.

Main benefits:
-playerside stats (saved along with player not separate)
-new lua hud statbar type bound to player stat
-slightly reduced network overhead (now equals same as old c++ statbars)

Midterm benefits:
-adding of new stats isn't core issue any longer
-clients may read information about other items too

Longterm benefits:
-stat values could be evaluated by client side lua code too

Open issues:
-compatibility
-strange behaviour on death (almost always within own bones)
-add support for lua entity stats too

{
// ignored
// TODO maybe add compat code
}

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja May 25, 2014

Member

These should definitely have compatibility code, at least for a release or two.

This comment has been minimized.

Copy link
@sapier

sapier May 26, 2014

Author Contributor

I just tried to add that compatibility and stopped at about 300 additional lines without even having full compatibility. I guess I'd need another 200loc to get this compatible. So it's adding about 300-500 lines of code ... quite insane considering the amount of code this is.
It's not worth doing a cleanup if you need even more code to re-add exactly old semantics. If we want code like this imho the only way to get it is breaking compatibility.

}

*/

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja May 25, 2014

Member

This should be completely removed.

* u8 len
* u8[len] statname
* s16 value
*/

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja May 25, 2014

Member

You should add deprecation warnings (// Obsolete) for TOCLIENT_{HP,BREATH}.

This comment has been minimized.

Copy link
@sapier

sapier May 26, 2014

Author Contributor

You're right

@ShadowNinja

This comment has been minimized.

Copy link
Member

ShadowNinja commented May 25, 2014

Seems good, but it looks like old player files will be deserialized with a health of 0, meaning that they will die when they sign in...

@sapier

This comment has been minimized.

Copy link
Contributor Author

sapier commented May 26, 2014

If I remember correct old players will get full health as they don't have a serialized stat element for health

@sapier sapier added this to the 0.5.0 milestone Jun 28, 2014

@ShadowNinja ShadowNinja added the WIP label Jun 29, 2014

@ShadowNinja ShadowNinja changed the title WIP replace hp/breath by generic stat mechanism Replace HP/breath by generic stat mechanism Jun 29, 2014

@sapier sapier force-pushed the minetest:master branch 2 times, most recently to c24e075 Aug 19, 2014

@ShadowNinja ShadowNinja force-pushed the minetest:master branch 4 times, most recently to 56195dc Sep 20, 2014

@sfan5 sfan5 added the Rebase needed label Oct 6, 2014

@ShadowNinja ShadowNinja force-pushed the minetest:master branch from cba038e to b98e8d6 Oct 7, 2014

@Zeno- Zeno- force-pushed the minetest:master branch to a1db83e Nov 25, 2014

@kwolekr kwolekr force-pushed the minetest:master branch to 3f83ca2 Dec 25, 2014

@sapier sapier closed this Jan 10, 2015

@sapier sapier deleted the sapier:generic_player_stats branch Jan 10, 2015

@sapier sapier restored the sapier:generic_player_stats branch Jan 10, 2015

@Ferk

This comment has been minimized.

Copy link
Contributor

Ferk commented Mar 27, 2016

What happened to this? why was it closed?
Even if it breaks compatibility, it would be a cool thing to have for an hypothetical 0.5.x, imho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.