Lighta

Hi so this is a suggestion to update our environnement. I'll talk about : 1) Main dir hirarchie and files in 2) Makefiles 3) Shell script update You'll need a basic knowledge on Make (Makefile), shell, and unix filesystem to understand the following. here the current diff : http://upaste.me/b5405421f849ddda 1) I'm thinking the main dir is kinda ugly with a bunch of file we don't use. (we don't use .bat and .sh in same time nor all .sln solution); I'd like to move each .sln into their project path and make a script folder for the (.sh .bat). Wich is more a less the same things as 'tools' dir atm, but I think a rename could be more disanbuisious and we could add more in their. Goal would be to cleanup thing a bit and to create a package/systeme rather then 1 prog like we tend to do atm with increasing map-serv while we could do other proc. 2) Makefiles So just a reminder of the current flow : configure.in is used with autoconf to produce configure, configure is used to produce Makefile by reading Makefile.in and finally make use those makefile to produce binary. With a quick look on Makefiles I saw we still had some 'txt' target in some place (src/map) wich had invalid target and therefore can't compile. You may have a binary with some few edit but it will most likely crash due to all the function require mysql. Therefor I think a small cleanup and harmonisation between those Makefile are necessary to prevent any of this attemp and to not have ugly Makefile harder to maintain. Also I'm not sure about the extensive check on mysql, sure it prevent compiling if you don't have the requirement by default but that would mean your ./configure return an error to begin with. Wouldn't be safer to check if configure didn't fail instead only mysql ? what if we add new lib ? I also change the 'all' target to run all the makefile in subdir so we could check if something didn't broke other like it's currently the case with src/test. (since Sql_Init) make install uninstall bin-clean was also rewritten and taken out as external shell script. I'll talk about them in part 3 3) Shell script. First we using shell script cause bash ain't the default interpreter in all distribution (ksh, tsh,..). athena-start : Since the .pid file was added to prevent killing other servers with the same name on same host. (like map-server p6900 and map-server p6800 dual server). There a small glitch issue if you attempt to relaunch start while a runnig server is on with the same config, it will fail (socket already in use) but the .pid file will be overwritten with the new process id wich won't correspond to the running server. So now we checking this before attempting start wich I hope will remove question about socket error 5 or etc. So new func would be : -status : return whetear the servers are up or down. -restart : now fully check if server down before attemting restart. -start : now check if no instance already running (will launch down ones if so) -watch : auto restart server if taken down (experimental atm) Also I'll save those action into log/map-server.log for exemple. With these change I think we could remove "char-server.sh, map-server.sh, login-server.sh start" wich the first 3 was only monitoring and the last was a multiple start attemp... install.sh uninstall.sh : Those 2 new shell are what was somewhat into make install | uninstall | bin-clean. There being extracted cause we usually have install script outside and also gave the advantage of having more instruction instead depend on make syntaxe. The previous install process was kinda like take our current repository and move it into install directory. (usually /opt/ or /usr/local/) While this "ok" it fork the emplacement of src npc was then in 2 different base path (origin path and install path) making change/sync harder and you couldn't reinstall without those folder into "origin path" (just imagine the nightware for a new script command..) Also "install path" are usualy under root ownership (security to prevent user to add non wanted apps..) wich wasn't check atm and result in a fail. (root ownership will also harder change a bit). I therefore prefer to copy file under installation path and make athena-start use it if we have so. -I wont tread /opt/ vs /usr/local/ that more an admin preference, let just say /opt will be easier to see as a whole package. -db/ was move from etc/ to var/ why ? even if in our case the db is read only it's still meant to be a db and idk maybe one day will writte in it too, db are usually in var/ for that reason not etc/ -Hierarchy of symlink change why ? yes wasn't make sense to have conf/ db/ etc into bin/ directory, but it's common to have binary linked into package dir, wich I did. Hierarchie of opt/rathena/ is the same as if that /opt/rathena/ was the / one. -rathena was added in usr/bin/ why ? this allow user to use "rathena start" everywhere as if it was in /opt/rathena, it's now treated as a real user system command. Why not add /opt/rathena/ into path instead ? didn't really want to bother with .profile and make uninstallation easier. (same go for alias) function.sh To facilitate script dev I moved common var and function into this file so it could be loaded and use everywhere. This prevent name mistmach for server declaration, install_path or package name etc..

ye I dont get it. open a terminal on your mac. port install subversion svn co https://rathena.svn.sourceforge.net/svnroot/rathena/trunk/ rathena/ cd rathena/ && ./configure && make sql cd tools/ && perl config.pl cd .. && ./mac.rathena-start.scpt that should do it, now instead taking rathena from subversion you could copy your repo and follow the same step to make it work. nb you'll still need "gcc ?","zlib-devel","make","mysql-server","pcre-devel (optional need for script defpatern)","subversion (optional)","phpmyadmin(optional)" package equivalent for mac. not sure if our configure or cmake produce makefile for xcode but you can install and compile with gcc on mac otherwise.

ye the atcommand is fine, issue is more to kinda define a pattern to we don't have script growing in every direction. yep sd was just to said player we could found them by name wich is fine for atcommand or (char_id). Atm no it's not possible for a player to have multiple guilde so it's fine to do it. If there was a change I believe we could extra location for guild2 guild3 etc, assuming they sorted... (anyway let's not bother with this yet). Last note is that for script even if we could resolde sd with nick I prefer via charid, but we could add both methode for flexibility. we tought same thing for loc =)

yes even tough not sure I really like the nomenclature of this. They must be kinda similar from script and atcommand (name) but we also don't want lot of script function atm. (well it's a same a flag could do the job). Think something more generic would be @countitem,@countitemstorage,@countitemcart,@countitemguildsto name doesn't matter that much for atcommand since we could do alias, but they more regxy like that and easier to maintain for script. (guildsto is similar to cart, if(sd as guildid)) countitem(id,{loc,sd}) with sd and loc optional, only bad part on this is for a regular countitem on a particular sd you'll need something like countitem(apple,0,lighta). now do we do each small to link to tht generic hmm ?

didn't saw that but I like the coding style, I may try it and review what we got to integrate it. nb : script would probably need them too.

yes doxygen but we need to reparse each function and add a comment on what it's purpose. also texte are nice but I'd really love some UML schema, modele and sequence, especially sequence since we more or less know the modele already. That would be a huge plus in our organisation to know where to put this or this handle..

you need libmysqlclient-dev, I used to have that issue too. I've put the package I uses to install in tools/config.pl if that help you to reduce some dependancy.

what your OS ? we need something like : http://rathena.org/board/tracker/issue-6561-unknown-map-server-crash/

that probably cause you mixed with eamod. we don't support eamod and well no one can really help you without a coredump or more info in this.

that warning mean pc_bonus3 receive a not handled SP_X (bonus type) and therefore throw you an error. You can look what this type is in : map.h, here for 1010 = SP_SUBELE, this 1010 could also be set in const.txt to be used in script/item use. Now that you know wich type is wrong you can look at pc.c what wrong and/or rectify src/item/script. nb: From warning message and item you quote I doubt it's one of them causing those warnings since you only using bonus and bonus2 in those itemscript while src stating it's a bonus3 fault.

Ryu is nothing about understanding or not. I have no doubt people understand it well, but that not mean it's longer to do so then a clean branch without those extra. Some of those #if have implicite relation like (struct item_data) orders declaration. Or thing like a status.val initialise on both but only used in 1 case. (yeah we could add proper #if check to make it proper but it's not the case). And if you get what I said image we doing so in a very well done handling, then you can magine as long dev continue; ifdef will increase giving a lessand less readable code. That really what he meant, nothing to do like can't understand or not... Now why readable matter ? on OpenProject like that many people contribute and if they can't read it fast they most likekly put code on a wrong place creating 2 handler for the same mean -> code complexion_> spaghetti... or just a general that long paragraph of mine make u lose more time to understand than a quite sentence. (time that you coul use to dig another bug etc..) But even if it's interesting for dev (even if that mean frequent extra merge work between both..) what really annoying imo is that people support/diff contribution etc will be impacted. Will at least require more info from support (wich branch/version if we could set it as prerequisit) and seing some say version=latest on bugreport doesn't feel to good about. (yeah you may found what was latest with date of report, but that not always their real rev...). For modules/diff well since index won't match obviously in both branch, you'll need most likely to do a version for each branch. (not that long but..) etc...

show us the result picklog table. Seem like the second query kyeme quote wasn't apply. As state "He didn't found unique_id colum in table picklog".

That why I said you'd need someone to handle that. (like pushing change from RE to PRE) etc... Also I pointed the last topic in my 1st post bouhou no one read me =). Making it clean is imo important for maintenance. Like you don't want to read hundread of preprocess #if. Wich will increase more and more time go on. Will make it harder for people to take it over and participate in project. (harder to search where was the correct handler..). Well you can if you will... But actually there's other project who will have better impact on project imo : - Documentations / Schema. (even if partially done it's still minimal and this may be good to review code and not have duplicate handler..) - Tests. (one thing that slow down major change is that we don't have regression test, we just try on our own commit and wait => this why we have quite significant reverts amount, 1 change break another etc... need like a QC. - Open it more. (git good for that for exemple). Many could contribuate occasonially but enter on "staff" may mean to much responsability time etc. (like right now I have no time and feel bad....). So yeah improve casual help. (danger might be we have too much mods and "pollute" repo too much... => In short probably require better guideline/milestone.). In comparaison branching would impact less I think, but all this is admin and we need more code done !!

Who's Stapo ?? You may ask but that annoying for people who face the same issue and would search for the awnser. If I get you quesion right you want to be ale to use warm on everymap wheter it's feeled or not; if so you need to inverse this block : case SG_SUN_WARM: case SG_MOON_WARM: case SG_STAR_WARM: if (sc && sc->data[sC_MIRACLE]) break; i = skill_id-SG_SUN_WARM; if (sd->bl.m == sd->feel_map[i].m) break; clif_skill_fail(sd,skill_id,USESKILL_FAIL_LEVEL,0); return 0; break; (like for exemple comment out the clif_skill_fail(sd,skill_id,USESKILL_FAIL_LEVEL,0); return 0; wich will then allow all to go trought even if not felled or in miracle. located in skill.c::skill_check_condition_castbegin l12678-12688 (switching skillid)

I'm not agreing with this : 1. This shouldn't be opensource neither in the server neither in the client, or it can be easily bypassed. Opensource doesn't mean bypassable and keep it closed will still present the failure if so just more difficult to find them/exploit. Or do you mean user need to hide their keys ? (sound obvious) Is that for all windows NT ? checking process running by name =( Don't get why he didn't do that in something more portative but it's a good side project wich probably give eA a regain of interest. (or people just gonna get lazy and complain). Anyway one of most wanted thing from user I think.

I do only cause I don't like all those preprocess macro for renewals : - They make function longer wich reduce readability => lower maintenability - Ain't fully validate by buildbot or really any automated tests (e.g r17082) (we should imo try a compile wich each define per week) but in other hand : - Many improvements also concern pre-re like new script_command, group system etc... - It will requiere someone to merge those change in the other branch even if that ain't so long * (also if could check other emu) -- Kinda the same as trunk/stable/tag actually we're on very basic versionning. - Support/bug/share may be annoyed since right now people don't often precise if pre or re and also work with line number. (in short mod will need 1diff for pre and re since line won't match..) nb : -- People will cry like A-style change since removing this will certainly broke many diffs --We can't summerize eA is the pre and rA renewal. Since that not the only in diff they got even if it's the more noticeable. (until kinda recent eA fix was integrate to rA but that stoped due to some generation of errors). --- Quite kinda the same as people saying BrA is same as rA but in portugese, they add thing on they're on too so it's not. ps : Precedent "debate" : http://rathena.org/board/topic/74215-p-re-vs-re-do-we-really-need-both/ even if the subject slide a bit.

that the normal behaviour, ye, but if you want it to continue while changing map you need to comment out : status_change_end(&sd->bl, SC_MIRACLE, INVALID_TIMER); in pc.c::pc_setpos (the whole changemap ending satus part atm l4672 r17086)

depend witch atk you want but you can take a look at : status_get_batk status_get_watk status_get_watk2 to get the battle-status atk of src and then alter the damage formula. (or sstatus->batk etc..) : http://rathena.sourceforge.net/doxygen/structstatus__data.html

ye look not so bad but vcc will probably complain for the devo_flag declaration not at begining of a scope. (so encapsulate all this with {} for easy fix) and you need to add a chk on tsc to avoid mapcrash, otherwise if src == BL_PET => tsc = null => tsc->data == segfault boom. rest look fine

That 2 question, you can't really said in other word. rnd() : /// Generates a random number in the interval [0, SINT32_MAX] % : modulo sign, return the rest of division => y%x = [0;x-1]

na but he shouldn't do that since mob/homon etc won't ever enter here then and won't be concern. (dunno who enter atm you need to test, but maybe that what u want yeah) otherwise jsut add : if(sd) blablabla; else if blabla else foo;

cause not only sd use that fonction I suppose. I should check but that my raw guess, you can validate it with the coredump. Try to add another if case for class_

done in r17072

guess we can add those but in something more general to not but overflowed with more and more npc command, trying to regroup them with topic. Beside that for issit isstand I think it's better to use the readparameter, (like weight, baseclass and such) wich allow to know the value directly in script. (from what I got this function is just made to know player info so would be better to continue there then to create new one)

you can but you'll need src edit to do so. 1) take charid as parameter on killmonter atcommand. 2) resolve to bl and transmit it to atkillmonster_sub (as second va_list arg is fine) 3) change status_kill wich have null as src to status_percent_damage() with the transmitted bl as src. This should do it : http://upaste.me/625a5141e5139904 [untested]