Author Topic: [fixed] Error morphing units with <resource-stored> tags  (Read 2033 times)

Ishmaru

  • Behemoth
  • *******
  • Posts: 1,066
  • um wat??
    • View Profile
    • DelphaDesign
I found a bug, that when you morph a unit (Unit A) that stores resources, into another unit that stores same resources (Unit B), the maximum amount of storage of Unit B is added to Unit A upon morphing. This can increase your maximum exponentially if you continue to morph between the two.

I think the proper behavior when morphing Unit A into Unit B is to use Unit B's Maximum Resource amount instead of adding the two together.

No errors stated or crashes.

Not to important info:
Windows 7, Nvidia G force, I7 processor, 6 gigs ram, running 3.8.0 Beta 1
« Last Edit: 11 October 2013, 14:51:14 by tomreyn »
Annex: Conquer the World Release 4 For Pc Mac + Linux
https://forum.megaglest.org/index.php?topic=9570.0
Annex is now on Facebook!
https://www.facebook.com/AnnexConquer

softcoder

  • MegaGlest Team
  • Battle Machine
  • ********
  • Posts: 2,238
    • View Profile
Re: Error morphing units with <resource-stored> tags
« Reply #1 on: 8 October 2013, 17:05:10 »
I think this is not really a bug as it is the intention of the design. However, we could add an optional setting for the morph command to indicate it should replace existing storage. How would you like to see that defined in xml?

Ishmaru

  • Behemoth
  • *******
  • Posts: 1,066
  • um wat??
    • View Profile
    • DelphaDesign
Re: Error morphing units with <resource-stored> tags
« Reply #2 on: 8 October 2013, 18:44:59 »
I'm not sure were talking about the same thing,
Just to be clear I don't want it to replace current total factions storage, the issue is that the storage value of that specific unit does not change properly.

The bug is this,

Unit A stores 200 gold
Unit B stores 500 gold

- When Unit A morphs into Unit B it (the unit) now stores 700 gold (adds 500 gold to unit storage)

- If I once again morph Unit B back to Unit A it(the unit) now stores 900 gold (adds 200 gold to the 700)

- This increases exponentially for each morph.

Proper Function would be:

Unit A stores 200 gold
Unit B stores 500 gold

- Unit A (stores 200 gold) morphs into Unit B, now stores 500 gold itself.

- Now Unit (currently stores 500) morphs back into Unit A, now stores 200 gold itself.

Hopefully this is a bit more clear, If you already were thinking this then I apologize.
Annex: Conquer the World Release 4 For Pc Mac + Linux
https://forum.megaglest.org/index.php?topic=9570.0
Annex is now on Facebook!
https://www.facebook.com/AnnexConquer

softcoder

  • MegaGlest Team
  • Battle Machine
  • ********
  • Posts: 2,238
    • View Profile
Re: Error morphing units with <resource-stored> tags
« Reply #3 on: 9 October 2013, 05:17:50 »
Yes i do understand what you are describing and the code was always this way (and possibly megapack might rely on this???). Your use case of morphing is not how the original factions were designed so that is likely why such an issue has not been discussed.

This is why i asked how we should specify in the morph command  whether or not we should add to or replace the units storage (really depends on the modders use case).

Omega

  • MegaGlest Team
  • Dragon
  • ********
  • Posts: 6,167
  • Professional bug writer
    • View Profile
    • Personal site
Re: Error morphing units with <resource-stored> tags
« Reply #4 on: 9 October 2013, 07:07:32 »
(and possibly megapack might rely on this???).
Are there really any legacy uses of this? I did a quick search for all morph skills in the megapack, and the only buildings that have morph skills do not store resources. I don't believe any non-buildings store resources.

Unless there's any mods that purposely depend on this behavior, I see no reason to add a quirks mode.
Edit the MegaGlest wiki: http://docs.megaglest.org/

My personal projects: http://github.com/KatrinaHoffert

Ishmaru

  • Behemoth
  • *******
  • Posts: 1,066
  • um wat??
    • View Profile
    • DelphaDesign
Re: Error morphing units with <resource-stored> tags
« Reply #5 on: 9 October 2013, 17:00:32 »
Sorry just wanted to make sure,

I think to specify in xml you can add a line to morph skill,

Code: [Select]
<overwrite_storage=true/>
Where true will be the new behavior.

Thanks softcoder!
Annex: Conquer the World Release 4 For Pc Mac + Linux
https://forum.megaglest.org/index.php?topic=9570.0
Annex is now on Facebook!
https://www.facebook.com/AnnexConquer

softcoder

  • MegaGlest Team
  • Battle Machine
  • ********
  • Posts: 2,238
    • View Profile
Re: Error morphing units with <resource-stored> tags
« Reply #6 on: 11 October 2013, 02:16:50 »
I added support for this to the command element (rev 4602):

<replace-storage value="true" />

tomreyn

  • Local Moderator
  • Airship
  • ********
  • Posts: 2,764
    • View Profile
    • MegaGlest - the free and open source cross platform 3D real-time strategy game
Re: Error morphing units with <resource-stored> tags
« Reply #7 on: 11 October 2013, 14:50:56 »
While I agree it's a weird design, this works as designed so it's not a bug, and I'm moving it to feature requests and tagging it [done].
atibox: Ryzen 1800X (8 cores @3.6GHz), 32 GB RAM, MSI Radeon RX 580 Gaming X 8G, PCI subsystem ID [1462:3417], (Radeon RX 580 chipset, POLARIS10) @3440x1440; latest stable Ubuntu release, (open source) radeon (amdgpu) / mesa video driver
atibox (old): Core2Quad Q9400 (4 cores @2.66GHz), 8 GB RAM, XFX HD-467X-DDF2, PCI subsystem ID [1682:2931], (Radeon HD 4670, RV730 XT) @1680x1050; latest stable Ubuntu release, (open source) radeon / mesa video driver
notebook: HP envy13d020ng
internet access: VDSL2+

· · · How YOU can contribute to MG · Latest development snapshot · How to build yourself · Megapack techtree · Currently hosted MG games · · ·

Ishmaru

  • Behemoth
  • *******
  • Posts: 1,066
  • um wat??
    • View Profile
    • DelphaDesign
Re: Error morphing units with <resource-stored> tags
« Reply #8 on: 11 October 2013, 22:28:49 »
I added support for this to the command element (rev 4602):

<replace-storage value="true" />

Thanks softcoder!
Annex: Conquer the World Release 4 For Pc Mac + Linux
https://forum.megaglest.org/index.php?topic=9570.0
Annex is now on Facebook!
https://www.facebook.com/AnnexConquer

titi

  • MegaGlest Team
  • Airship
  • ********
  • Posts: 4,224
    • View Profile
    • http://www.titusgames.de
Re: [done] Error morphing units with <resource-stored> tags
« Reply #9 on: 16 January 2015, 22:56:43 »
We had the same trouble ishmaru described here with teh new prax_next mod.  I don't think we did understand ishmarus problem back then. we now added a working and simple fix in git .

The problem was the following:
if you have one building with a storage of 500 and you morph it into a building with 600 storage your faction has a total storage of 500+600=1100 also you only have one building with 600.
This is is the bug Ishmaru described. When you now kill this building your faction still has a 500 storage because just the actual storage is remove form the total.
We simply remove the storage of the former unit after we added the storage of the new unit.

I also removed the "replace-storage" switch because its no more needed. Beside of this no one used this switch and I cannot imagine that replacing the whole factions resource with the one of the new morphed unit does make any sense in any mod.
( https://github.com/MegaGlest/megaglest-source/commit/667b043393ace7c0a28bfc5988960d2f25b238ac )
« Last Edit: 16 January 2015, 23:09:24 by titi »
Try Megaglest! Improved Engine / New factions / New tilesets / New maps / New scenarios

Coldfusionstorm

  • Golem
  • ******
  • Posts: 867
    • View Profile
Re: [done] Error morphing units with <resource-stored> tags
« Reply #10 on: 13 March 2015, 01:05:46 »
We had the same trouble ishmaru described here with teh new prax_next mod.  I don't think we did understand ishmarus problem back then. we now added a working and simple fix in git .

The problem was the following:
if you have one building with a storage of 500 and you morph it into a building with 600 storage your faction has a total storage of 500+600=1100 also you only have one building with 600.
This is is the bug Ishmaru described. When you now kill this building your faction still has a 500 storage because just the actual storage is remove form the total.
We simply remove the storage of the former unit after we added the storage of the new unit.

I also removed the "replace-storage" switch because its no more needed. Beside of this no one used this switch and I cannot imagine that replacing the whole factions resource with the one of the new morphed unit does make any sense in any mod.
( https://github.com/MegaGlest/megaglest-source/commit/667b043393ace7c0a28bfc5988960d2f25b238ac )

I agree this should be considered a bug. ThubgsUP for fixing it proprely Titi  :thumbup: :thumbup:
WiP Game developer.
I do danish translations.
"i break stuff"