MachineMuse / MachineMusePowersuits

Minecraft mod, take 2
236 stars 105 forks source link

1.10.2 problems #828

Open eyeonus opened 6 years ago

eyeonus commented 6 years ago

I noticed a couple things while playing with the suit on 1.10.2

Firstly, the binoculars module doesn't work at all. I'm not sure if it's because I don't have Optifine installed, or if it's something else.

Secondly, even without any armor of any kind on, I don't take damage from fire or lava. Other damage does happen, so maybe it's because I have the water bucket cooling module installed?

Thirdly, the texture for the water bucket cooling module is broken- it's the pink and black squares.

lehjr commented 6 years ago

Interesting. Unfortunately there are probably more than a handful of bugs that really need to get fixed. Some of it is just not enough testing.

eyeonus commented 6 years ago

Yeah, I figured. No worries. I mostly put it up to let you know, I'm not concerned about how long it'll take to fix. I know you're busy with the 1.12 port.

lehjr commented 6 years ago

While working on the 1.12 code, I noticed a weird code branch for the PowerArmor which is probably the cause of the issue with the lava damage not working. https://github.com/MachineMuse/MachineMusePowersuits/blob/1.10.2-Java/src/main/java/net/machinemuse/powersuits/item/ItemPowerArmor.java#L60-L73 That should actually be this:

        if (source.isFireDamage() && !(source == MuseHeatUtils.overheatDamage)) {
            return new ArmorProperties(priority, 0.25, (int) (25 * damage));
        }
lehjr commented 6 years ago

Basically the way this is supposed to work is that the heat damage doesn't take effect until after the max temperature is reached. That part is working from what I can tell. However, the cooling seems to be working so fast that the armor does not have a chance to heat up. Also, from what I can tell, the heat sink modules aren't even doing anything at the moment.

lehjr commented 6 years ago

Currently, the only way to even rack up enough heat to take damage from lava is to be over your head in it.

eyeonus commented 6 years ago

The only difference I see in the existing code is that it has "ISpecialArmor.ArmorProperties", otherwise it looks the same.

But awesome that you found the problem. 👍

That reminds me. Another problem I found, the auto-feeder doesn't update saturation. MY saturation has been at 0 consistently ever since I installed that module. Looking at the code, it seems that it turns the saturation into hunger bar filler? Am I reading that right? (Also might want to get rid of the "old" feeder since there is no config option for it and I doubt anyone wants the eats the whole stack version anyway.)

lehjr commented 6 years ago

I did some experimenting. The heat sink modules are working, but all they do is raise the max temperature before the overheat damage starts happening. Part of the problem is in here, specifically the checking of if the player is in lava. I rewrote the whole thing to look like this:

    public static double getPlayerCoolingBasedOnMaterial(EntityPlayer player) {
        if (player.isInLava())
            return 0;

        double cool = ((2.0 - getBiome(player).getFloatTemperature(new BlockPos((int)player.posX, (int)player.posY, (int)player.posZ))/2)); // Algorithm that returns a value from 0.0 -> 1.0. Biome temperature is from 0.0 -> 2.0
        if (player.isInWater())
            cool += 0.5;

        // If high in the air, increase cooling
        if ((int)player.posY > 128)
            cool += 0.5;

        if (!player.worldObj.isDaytime() && "Desert".equals(getBiome(player).getBiomeName())) { // If nighttime and in the desert, increase cooling
            cool += 0.8;
        }

        if (player.worldObj.isRaining()) {
            cool += 0.2;
        }
        return cool;
    }

which is now using a different and more precise check for lava and the code is working better. However, the cooling code is still fired too often.

lehjr commented 6 years ago

auto feeder? Interesting. That I'll have to look at some other time. For 1.12, I'm going to make the heat sink modules reduce the amount of heat rather than increasing the max heat. I'll probably make the overheat damage increase with the amount of overheat if possible. There are really a lot of different things I want to do, but realistically, time probably won't allow half of it.

lehjr commented 6 years ago

I did some more testing, I think I can do a quick and dirty fix for this by reducing the cooling by 75% https://github.com/MachineMuse/MachineMusePowersuits/blob/1.10.2-Java/src/main/java/net/machinemuse/utils/MuseHeatUtils.java#L36-L56

Basically just add a line coolDegrees = coolDegrees * 0.25; in the first line of that. I did test it and it does still build up a slight amount of heat with the liquid nitrogen cooling system. Since they do stack, it's probably good enough.

eyeonus commented 6 years ago

Rather than having the modules affect the amount of heat, why not have them affect the rate of cooling, as a percentage increase, the same way that being in water gives a 50% increase?

lehjr commented 6 years ago

I'm still not really sure how I'm going to do it yet. The biggest issue with heating and cooling is that they happen at significantly different rates. Heat is added through the armor damage mechanic and only fires every second or so while the cooling mechanic I believe fires every tick, or at least several times more often than the damage mechanic.

eyeonus commented 6 years ago

That explains the huge jumps I see in the heat meter sometimes.

I have an idea: Divorce the heat from damage, (isn't that what the armor is for anyway?) and make it a per tick thing like the cooling. Have each module have a heat generation rating based on how much power it uses, and increase the heat by that amount when they are on. (Or used, in the case of the Fist's modules.) Give each piece a default cooling rating if that hasn't been done already. Optionally give the armor plating modules a higher cooling rating.

lehjr commented 6 years ago

The mechanics are a bit complicated. There are several mechanisms in place rather than a single one. The one I was referring to was for the heat based damage for the armor being in a heat source, like lava or fire. That only fires once every second or so. Overheat damage is routed through that. There may be other routes for it as well, I haven't really checked. (Currently fixing up the FOV code a bit).

As far as the cooling ability of armor plating, there are already 3 cooling modules available each using some kind of resource. The plating itself would have more of an insulating characteristic, which is why I was considering reducing the amount of heat it allowed through. But keep in mind that the weight mechanic is gone in 1.12, and with that, any kind of a thickness setting. It's more likely the mechanic will allow multiple plates to be installed, each adding to a total value of whatever properties it has.

lehjr commented 6 years ago

Binoculars issue seems to be related to the FOV handler code in Numina. Once I fixed that to how it should work, the issue is still there to some degree. Basically, the binoculars will only work if the FOVFix is turned off. There's a keybind setting for it, but it wasn't even working because when I refactored the code the last time, I didn't spend enough time testing it. So when a player would enable or disable it using the keybind, all they were doing was sending themselves a chat message. In order to fix this properly, this will need a major refactoring, moving that code out of Numina and into MPS so that there aren't 2 competing FOV Event handlers. Also need to add some code to restore the setting state when the player logs back in instead of having a default value.

lehjr commented 6 years ago

oops

lehjr commented 6 years ago

There, fixed it without refactoring. :D

eyeonus commented 6 years ago

It makes sense that overheat damage is routed through that, but not that heating itself is.

Giving the armor an insulation rating makes sense, I like it.

eyeonus commented 6 years ago

lol

lehjr commented 6 years ago

Config option was missing because it wasn't being initialized on startup. I fixed that part, but the saturation thing will need to be explored further.

lehjr commented 6 years ago

The autofeeder seems pretty broken to me. It's only supposed to consume food if the player is hungry, but it just consumes all of it. I'll have to fix that before I upload a new build. Might be the last 1.10.2 build for awhile.

eyeonus commented 6 years ago

I think you're looking at the old method. The old method consumes the whole stack, the new method only consumes into the player is full, and any left overs are added to the buffer.

eyeonus commented 6 years ago

*until

eyeonus commented 6 years ago

In my game I've been keeping blueberries from Tinker? Natura? Can't remember, that give one point each, and the module's only been grabbing them at need.

lehjr commented 6 years ago

Gonna see if I can get this autofeeder fixed today, then that will probably be it for awhile for 1.10.2 updates.

lehjr commented 6 years ago

Both the old and new method have the same problem of calculating how food and saturation are used. Everything is done around hunger and food consumed and saturation doesn't get added to the player stats unless food is consumed. Both of these need a rewrite.

lehjr commented 6 years ago

After some experimentation and digging, I think I have it all figured out in terms of how to solve it. I'll put it in the next commit.

eyeonus commented 6 years ago

Are you going to be pushing this to Curse after the commit?

lehjr commented 6 years ago

yes. It will probably be labeled as a final build or something similar. It probably won't be an actual final build, but I probably will only be doing one or 2 more as maintenance releases.

eyeonus commented 6 years ago

Cool beans.

Also, found another problem: the rototiller is bugged. It has a tendency not to till the dirt block you're clicking on, in that it almost never does, and while it does till the surrounding blocks with the radius set high enough, it won't do any dirt that it previously failed to till.

lehjr commented 6 years ago

Unfortunately, it will have to get fixed some other time as I've already spent days on 1.10 and I'm falling further and further behind on 1.12. I still don't even have the modelSpec code finished yet. So after I fix the autofeeder and upload the new version, it's back to 1.12.

eyeonus commented 6 years ago

No worries. I'm sure most of these issues are in 1.12 too, so it's not like it's a complete waste.

lehjr commented 6 years ago

The plan is to rewrite much of the module code, so most of it won't carry over if all goes according to plan. Much of this is because of the modules use doubles for data types for tracking attribute settings that in many places could be reduced to a byte with little to no impact.

eyeonus commented 6 years ago

Yeah, I hear you. I think most if not all should be at most an int or short.

lehjr commented 6 years ago

The biggest concern for data type sizes is for the network traffic. I mainly just want to reduce the packet size. I did add another packet type for config syncing though: https://github.com/MachineMuse/MachineMusePowersuits/blob/1.12.2/src/main/java/net/machinemuse/powersuits/network/MusePacketConfig.java

That's for server side settings and allows the client to get the settings from the server. I got the idea from looking at Scanable's code.

eyeonus commented 6 years ago

Super cool. Packet compression is definitely a good thing.

eyeonus commented 6 years ago

Since the update, my food buffer has been at 17 and never goes up or down, and no food is being used no matter how much I get hurt, run, etc.

lehjr commented 6 years ago

Fantastic. Unfortunately, probably won't get the chance to look at it until the weekend.

eyeonus commented 6 years ago

No worries.

lehjr commented 6 years ago

Found a few issues with the code and fixed them. However, with everything working now with tracking food and saturation separately, there is now an issue of food the food buffer accumulating a large amount of food to keep the player saturation full. So player sees the food level increasing and yet is still consuming food. This is because even during healing, the player's food level doesn't drop until the saturation level runs out.

I was thinking about combining the food and saturation buffers of the auto feeder into a single food buffer and then using that to fill both food and saturation stats as needed. This way, there's no wasted food accumulation and the player isn't left mystified on why they're still consuming food. It would also remove an NBT tag from the helmet. I have another bug to look at before I upload the fix. So I'll what you think on that.

eyeonus commented 6 years ago

I don't think it's necessary to make sure the saturation bar is filled, as long as the saturation of the consumed food is applied. You have to have a mod installed to even see your saturation level, so most probably wouldn't even notice.

Also, in Vanilla Minecraft, you can't eat if your hunger bar is full, regardless of how high or low the saturation bar is.

As such, I see two possible solutions: 1- Apply the saturation level of the food consumed, in full, when the food gets consumed by the module. Don't even bother with having a saturation buffer. This does mean that the saturation bar doesn't increase when refilling the hunger bar from the food buffer, and it is possible that some saturation would get wasted because of that, but I doubt it'd be very much.

2- Apply the saturation, and have a buffer, but don't try to keep the saturation bar filled. Basically the same as above, except any excess saturation goes into a saturation buffer, and any excess saturation is used to fill the saturation bar as needed. More food is consumed only when needed to fill the hunger bar.

eyeonus commented 6 years ago

(If I'm reading the code right, option 2 is achieved by simply changing line 93 to if (foodNeeded > foodLevel) { and removing the saturation check there.)

lehjr commented 6 years ago

Doing that would essentially return the functionality to what you were complaining about in the first place with the player saturation level always being bottomed out. Saturation is always used up before the hunger bar will ever move.

lehjr commented 6 years ago

Technically, though, it's not wrong to do it that way. A player can only eat if their food level is below 20 regardless of their saturation level. It would mean that this section would have to get merged into the food stuff above it: https://github.com/MachineMuse/MachineMusePowersuits/blob/1.10.2-Java/src/main/java/net/machinemuse/powersuits/powermodule/misc/AutoFeederModule.java#L138-L160

eyeonus commented 6 years ago

Not really, because before the code changes, the saturation bar wasn't changed at all. It was always at 0 because either the saturation of the food consumed wasn't used in the code at all, or it was applied to the food buffer. I forget which.

The saturation bar would be empty before the module ate anymore food, due to the hunger mechanics of Minecraft, but it wouldn't always be 0, because any time a new piece of food was consumed, the saturation would go up by the amount of saturation that food has.

For example: Before consumption: sat = 0 (empty) hunger = 12 (4 empty hamhocks) food buff = 0 sat buff = 0

food consumed: "Sardines in Hot Sauce" from Pam's HarvestCraft x 2 Hunger filled: 6 Saturation filled: 19 (total= 12 hunger, 38 sat)

After consumption: sat = 20 hunger = 20 food buff = 4 sat buff = 18

eyeonus commented 6 years ago

Ah, yes, I see that now. I didn't really look into that part of the code, because I thought it was where the bars are being refilled from the buffer, and the filling of the saturation bar from the buffer is probably better kept separate from the hunger refilling.

lehjr commented 6 years ago

saturation goes up when a player eats, but it crashes really fast, especially when healing or even just running. So going back to the original behaviour would mean that the saturation level is rarely more than zero just because of how fast it drops. It's really oddly coded in Minecraft, but it was never meant to be interacted with directly anyway.

eyeonus commented 6 years ago

That's fine. Using saturation in the module the way Vanilla does is perfectly reasonable. It's the not using saturation at all that I was marking as an issue.

lehjr commented 6 years ago

OK, I've taken the saturation checks out and moved the saturation consumption. Now to see what this does.

lehjr commented 6 years ago

Yeah, good call, that seems to be more in line with how it should work. Food is actually being consumed at a more "vanilla" pace and the food buffer isn't loading up while consuming food. I'm not really tracking the saturation buffer, but I would assume the behaviour is closer to vanilla given how slowly the hunger bar drops.

eyeonus commented 6 years ago

Found a bug.

In both the Food buffer and Saturation buffer filling code, in the case that needed is greater than level (first check failed), the code is checking to see if "(needed - (level 0.5 energyConsumption)) < suit energy", and if true setting "used = needed - level", which is definitely not what we want. (Parentheses added by me to more clearly show how the expression evaluates.)

For example, when level = 0 and needed = 4, the check is "4 - 0 < suit power", and used gets set to 4, even though there's nothing in the buffer to use.

https://github.com/MachineMuse/MachineMusePowersuits/blob/af5698b70b193f8a4497237c55f840fbb5ff87c6/src/main/java/net/machinemuse/powersuits/powermodule/misc/AutoFeederModule.java#L119-L120 Change to:

            //Need more than is available, check if enough power to use all available.
            } else if (MuseItemUtils.getFoodLevel(item) * eatingEnergyConsumption * 0.5 < ElectricItemUtils.getPlayerEnergy(player)) {
                foodUsed = (int)MuseItemUtils.getFoodLevel(item);

That is, remove "foodNeeded - " from both lines.

https://github.com/MachineMuse/MachineMusePowersuits/blob/af5698b70b193f8a4497237c55f840fbb5ff87c6/src/main/java/net/machinemuse/powersuits/powermodule/misc/AutoFeederModule.java#L143-L144

Same thing, but "saturationNeeded - "