Hi, my name is Ori. I saw your project and am very interested. How can I help? I have a sourceforge account (tohava), I successfuly compiled and ran VCMI, I know C++, and am a Linux user.

Is there an issue tracker for this project? Because the sourceforge one seems outdated. Are there any easy bugs a beginner like me can start with?

tracker

And wiki

Hi!

Any help is warmly welcomed.

I’ve prepared such guidelines several months ago and it’s still valid. See [forum.vcmi.eu/t/h3-vcmi/327/30) and discussion below. You could for example take ungureanuli’s patch and address mentioned issues.

sorry for being slow to respond, pressure in job… I’ll get to work on this in friday…

I have patched bugs 64 and 37 (for 64 I used a shorter and different fix than the one in ungrueanuli patch), tomorrow I will probably handle 27 as well (I know ungureanuli did it but I prefer to go over his change first). Since you told ungureanuli to leave 69, and 48 is already closed, I would like to know if there are any other issues you would recommend me to go for next. If not, I have noticed an animation bug that occurs whenever a unit attacks another one from behind (i.e. attacker attacks from right instead from left as usual), I might try to look into it.

Also, I have some questions:

• I have noticed in many places in the code you use numbers directly instead of constants. Why is it so?
• You have nice functions like vstd::find and vstd::contains that could probably be used in much more places instead of uglier longer calls. Would you like me to go over the code and try to refactor as many of these as I can?
• During combat, vcmi needs 100% CPU while waiting for player to move his stack, why?

Regards, OriB.
fix_64_and_37.patch.gz (1.08 KB)

Nice patch. You can fix bug 69 too if you want but it’s very minor and I think it would be hard to precisely reproduce ‘the Heroes III feel’ here.

Regarding the bug you noticed, feel free to fix it. If you want to fix something bigger, you can take a look at CRANIM.TXT file from heroes 3 and think how it could be used. If you want something simple, take 694 or 685. If you want to investigate something mysterious, then check my attempt to remove DefHandler (structure for a set of graphics) from CGDefInfo (rev 1904 reverts relevant changes). For some reason, more elegant solution with graphics in client’s code made adventure map displayed incorrectly.

Most of them are coordinates of graphics/buttons or other numbers used only in one place. I don’t think we would benefit from separating that data from code. However in some places constants are justified and we just were too lazy to put them – feel free to fix it.

If you want, you can refactor it but I think there are more important things to be done.

IIRC the code is just a bit inefficient… recent introduction of bonus system caching should have done something with it. I’m not sure what exactly is the problem right now.

Committed, thanks for contribution!

However, the reason for 100% CPU usage was in your patch — the battleGetAvailableHexes are very expensive, we can’t afford to call it each frame for each stack. I’ve extracted the problematic bit into a separate method, so the rest of patch can stay and be of use when that issue is solved.

Not so long time ago I’ve learnt that Boost.Range has a set of STL functions in that nice form accepting the container itself for a range.

I’ve fixed 37 in a way that will use less CPU, I’ve also added a fix for 27 as well as solved the mystery problem with handler removal. The mystery problem is the result of the (id,subid) pair not being a unique identifier for the graphic.

EDIT: I will continue with working on bug 600
fix_27_37_handler.patch.gz (5.08 KB)

sizeof(bool) is implementation dependent, so that memset is not correct. size should be BFIELD_SIZE*sizeof(bool).

Handled ubuntux’s comment,
also added fix for bug 600.
fix_27_37_600_handler.patch.gz (7.49 KB)

Hi OriB,

Thanks for the contributions. I’ve reassigned #27, 37, 64, 69 & 600 to you. #48 was closed as we couldn’t reproduce it anymore. If you still see the issue and have a fix for it, I can re-open it an assign it to you. Or you can do it yourself because (btw) I granted you access to change assignment/status of reports on MantisBT as of now.

FYI - #37 is only partially fixed, because the boxes are still not in the same place as in H3, plus in some cases they are totally or partially overlapped by obstacles or the creature itself, and they don’t move as they should when two friendly creatures are in front of each other. I’ll add a more detailed note on this on Mantis.

Cheers,
Zam

ori.bar, I don’t have up-to-date setup of VCMI (too many interesting things to do… VCMI has to wait) so I cannot test your patch but the changes look well. I can grant you direct access to our svn if you give me your sf.net login.

Zamloxis: regarding friendly stacks in front, the box positions are only updated when it is the turn of a human stack, so if you move one of your stacks in front of the other and the next stacks to play are computer stacks, the box will only move when the next human stack begins to play. Is it possible you did not wait for it?

ToW dragon: tohava is my sourceforge user.

Done. You are now a developer on our bugtracker too.

Well, first of all, the update for the box positions shouldn’t have to wait until the next turn of a human stack, but should be done as soon as the creature assumes the static position in the new hex. The order should be the following:

• creature’s turn comes > stack box still displayed
• wait/defend on the creature > stack box remains unchanged and another creature gets its turn
• if instead the creature moves/shoots/casts, the box should disappear until the action is completed
• the box should reappear when the creature finishes the move/shoot/cast action
• if at the end of the move the creature arrives in a hex adjacent on the horisontal to the hex of other creature or obstacle, in such way that its box would appear on top of the other creature or obstacle, then the box should appear in the (front) hex of the creature instead. Together with that, if similarly necessary, the stack size box of the other creature should move in its (front) hex. And all this already before all the below.
• if at the end of the move the creature attacks, or arrives in a hex with moat/mine/etc that causes damage, the box should disappear again during the attack/spell animation, and reappear afterwards, updated with the new stack size if the case. In the same time, also the box of the creature receiving the attack should disappear while the “damage receiving” animation happens, and reappear (updated) afterwards.
• if the result of the attack is the termination of a target stack in front, the box reappears (updated) in the front hex again (on top of the corpse)
• if after the attack, the creature receives a retaliation (physical or fire shield type), the box should disappear again during the retaliation animation, and reappear (updated) after
• if the result of the retaliation is the termination of the attacker, the box of the retaliating creature should move back in front of it, if previously was temporarily moved in creature’s hex
  …and so on. I’m sure I probably still missed some particular situation, but the above should cover the most. And the idea is that the stack size box should be updated (number and position) at the end of each active animation, meaning we may end up having it popping in and out in different places even during creature’s turn. And definitely should not have to wait for the turn of another human creature to appear in the right spot.

Now, all the above was to clarify the rules of when should the box (position) be updated. Another thing is the fact that, corrections aside, the placing of the box doesn’t yet work like you state above either. I can wait for the turn of another human creature, and the boxes are still overlapping other creatures, or even the boxes of other creatures (very little difference to the buggy behavior in 0.85). See the new screenshot & updates to my note for #37. We can see in the screenshot that we are in 0.86, and no box of the 4 creatures on top (Pegasi, Sprites, Dragon & Unicorns) is where it should be. If for you it works differently, I can only suspect that Tow perhaps committed your first fix to #37, while in your second attachment here there are further updates to it, so what we have in 0.86 is not your final version of the fix.

Oh, I thought you were also refering to my second patch (attached here not yet commited). This explains things. You are right, the first fix is not enough. The second still has the timing issue, but handles the position better.