Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[10683] Revert "[10677] Send to creature/etc Update call real diff fr…
…om last update and use it." This reverts commit 10784a8. Main reason: impossibility for me as commiter test problem and fix all corner cases problems.
- Loading branch information
VladimirMangos
committed
Nov 5, 2010
1 parent
d375d7d
commit 5ffa34a
Showing
35 changed files
with
149 additions
and
162 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?!! Good idea...
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да уж... Сначала подгонял патчи под этот коммит, а теперь обратно...
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да причем тут подгон... идея-то верная. а мелочи - поправим.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я смайлик забыл добавить =) Идея, скорее всего, верна, но что-то я не особо понял, что это должно было дать на практике. Да нам в принципе, рса, можно и не брать в расчёт этот коммит - ревертить этот реверт =)))
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think i write in commit note reasons.
I at 100% agree that idea good but i can't fix it without testing and reproduce. :(
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is no reason to swing the Frostmourne, the more so because the problems are found and solved.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the patch is out there, and anybody is free to provide tests and fixes such that this change can be reapplied ;)
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where? I get only insider42 test results and my second patch (in gist currently) not help...
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is gist for reference: https://gist.github.com/664145 i will not drop it some time.
If someone at linus test it more deep (better with less custom changes as possible) and confrm that at clear code it fix porblems or suggest some additional fixes for resolve still existed problems, i not have anything against reapply revert, gitst patch/alt + fixes. if this all in sum will work and fix in clean way problems.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solve (published in original commit) and discussion (also with Insider42)-
https://github.com/rsa/mangos/commit/e8b1a653d07801fdfea81de2e0352594fefea5fd
You searched for the problem in the wrong place.
PS server with this (and many-many other) patch have 20 hours uptime, no problems with GO/creatures.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rsa i not meaning as fix your horiable hack written wihout attempt understand real problem.
My patch in gist fix real porblem with wrong "future" msecs time in object last update field. But not resolve all porblems.
You pointed "fix" just "recorrupt" broken data. signess no meaning in msecs time. And for diff also. Your "negative" values just long diff in result unexpected "future" time in left arg. getMSTimeDiff just not created (as any diff calculation in fact) for cases when next time less that old time.
But in uint32 range not negative values, all values can be correct diff just long.
You talk about like fix of problem as "complete fixed" then for me nothing disscuss with you in this case. Just lol.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, you have not tried to understand the real problem, to illustrate the location of which (and not for use in your code), I proposed this "horriable hack". Those modifications that are made in your gist, are irrelevant to the problem.
The problem is that it is not in the code, but my head developer patch. All data supplied to the code UpdateCall completely correct. There are no broken or corrupt values. There is no "next time". The fact that the object is created from a single thread (and m_lastUpdateTime initialized from there, see WorldObject constructor), and UpdateCall called from another. And the time in different threads of any multi-tasking (except for real-time) OS has every right to vary within a single system quant in any direction.
The second part of the reason (may appear as a separate and complete with the first one) is that object creation can be completed much later than would be caused by UpdateCall for the entire group of objects, which includes the newly created. But it depends on the compiler.
As a result of these reasons, calculated time difference will be negative.
solutions:
Completely correct solution (if I understand your point of view) should be to implement the initialization m_lastUpdateTime by values, passed thread WorldUpdate the first round objects, including the newly created. However, full implementation of this solution is very difficulty, includes a new thread with the reference time and complex inrerprocess communication.
And please, do not be nervous. If the discussion does not like - just do not keep it, and everything will be okay.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw a lot of variants of 'fixes', but one little question: who will test all those variants?
As Vladimir said, he cannot reproduce problem(me too), so its impossible to fix it by self
If you can fix it in correct way - just do it
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which means "correct path" in your understanding? I suggested four correct (in my sense) solutions to the problem and explain its cause. if you think that the problem in another - I'm ready to agree with you, if you get a strong case.
I can configure the system (and compiler) so that the problem manifested itself, and so it did not manifest itself. if you can not replicate the problem, a description which I gave - it's your lack of knowledge rather than objective reality.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wtf
rsa make good patches + hacks ( works perfect )
@VladimirMangos
your team are very slowly,
why ?
only examples:
BLINK for mages or Mirror ( about 2 years, your team not fixed ) works perfect on RSA
Verhicles not implented about 1 year, works perfect on RSA core
Random BG not implented, 1.5 years, works perfect on RSA
Dungeon Browser not implented 1 year. ( works perfect on trinity core hehe )
u say your core is stable ? haha why u used a buggy ACE 5.8.2 and why runaway alot of devs from u ? ( trinity core now the devs )
u say your code is clean, the code is not clean !
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rsa, how your multi-threads related to clean mangos sources? Object created in same threads that Map::Update run.
If muti-threading will be added to mangos core it will be per-Map mutitreading and then Map::Update still will called in same thread as object creating.
Including any players actions in world in same Map thread context. Maybe i has been wrong revert base at test in broken design custom multi-threaded patches... But hard for me be sure when only single man provide test results for problem that i can't repeat at accessable platform and he use in same time same broken by design multi-thread patch (if you right in own description different threads use for same Map activity.
Ofc, if porblem only in used multi-threads support changes then in this part this is not mangos devs problem and you free fix it in any horriable ways select. I can reapply commit and apply my second fix that fix real problem repeated at clean sources and not related in any way to multi-threads: fix for case when object created in Map::Update call context, so after fixed update time send to objects.
Your negative check in any way wrong independent from how wrong value generated. All 0..0xFFFFFFFF values in msecs is valid values.
getMSTimeDiff strongly expected have in right arg future value in comparison left arg. And i think you not understand that in case msecs timer
0xFFFFF123 can be correctly in past for 0x0000000, because msecs timer overflow is normal functionality and getMStimeDiff specially writed for return proper (past,future) pair diff in like cases.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Betaman2k, fuck off. You garbage post not related to discussed problem. And you as i think mixing devs and contributers.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rofl
latest core from RSA and about 300 testers about 20 hour uptime, that is good, no crashed with your infos
and why u used bad words, that is a true story
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Object created in same threads that Map::Update run
it's true only for windows. objects in GNU based *x created in another libc/vsyscall thread (if not use special cases). internal mangos multithreading nothing to do with this.
my second "horriable hack" - https://gist.github.com/665285 don't use negative checks, but work also good. based on logic, when i write below.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rsa, i glad to heard that you can configure your own compiler
and, stop talking about multithreading, clean mangos sources has no MTMaps,
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version at least make sense. It will redundent applied in 0 timer case when 0 value is corret current timer value bit in any case will no have wrong effects.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
на win32 проблемма не воспроизводима
yes. win* not have real multitasking.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vladimir - this version also hack. Need special etalon timer in separate thread, and time value not must be sended in chain of update() calls, but must be getted from this etalon time thread. but this solve is very difficult.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm interested in how internal multithreading on *nix works. rsa, can you give smallest possible example to demonstrate it (I'll test it on Ubuntu 10.04) or give a link to explanation?
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
teyrnon - problem - not from modifications (reproduced on a completely clean core). problem from a misunderstanding of the principle of multi-tasking (which is not surprising for windows-users)
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zergtmn - this discussion is very good example :) example what character do you want?
main{
sprintf(thread_num1)
new _object
}
_object{
sprintf(thread_num2)
}
in win* thread1 == thread2. in GNU *x - depends from compiler keys.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rsa if some option make new call executed in independent thread then this not compatible with expected way work in mangos code. Just not use this option.
Normal C++ code expect that after return from new call object creation finished. Above i specially point at expected way code work: single thread for Map called code without any addition threads... In fact i not care about problem with your discribed way work. So we can maybe agree both then: this not mangos core problem but your "configure the system (and compiler)" specifics. Different from normal expected code work. And unsupported. We not plan in any future per-Object threads for creatining or any other purpose.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
em, how multitasking related to mangos..?
mangos have only one process and few threads
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's default case in GCC (over 4.0 if I remember correctly. have not watched the changes). if your want to use special cases - need include this in compiler settings.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rsa, what gcc options do you use?
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How this can be default case if lot existed code expect totally different way work.
5ffa34a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot reproduce with default compiler options and even with -O3.