-
-
Notifications
You must be signed in to change notification settings - Fork 34
feature_WorkerExpGainedOnCraft #1178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
7be1f2a
b2c7569
acd4f5c
287fba8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -342,7 +342,7 @@ TieneObjetos_Err: | |
| Call TraceError(Err.Number, Err.Description, "Trabajo.TieneObjetos", Erl) | ||
| End Function | ||
|
|
||
| Function QuitarObjetos(ByVal ItemIndex As Integer, ByVal cant As Integer, ByVal UserIndex As Integer, Optional ByVal ElementalTags As Long = e_ElementalTags.Normal) As Boolean | ||
| Function QuitarObjetos(ByVal ItemIndex As Integer, ByVal cant As Long, ByVal UserIndex As Integer, Optional ByVal ElementalTags As Long = e_ElementalTags.Normal) As Boolean | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You updated: QuitarObjetos(... cant As Long ...) But left: CarpinteroQuitarMateriales(... CantidadElfica As Integer, CantidadPino As Integer) If someone crafts in bulk and CantidadElfica or CantidadPino can exceed 32767, that’s a silent overflow risk. Action: make all these quantities Long for consistency: CantidadElfica As Long CantidadPino As Long (and any other “Cantidad*” used in bulk crafting paths) |
||
| On Error GoTo QuitarObjetos_Err | ||
| With UserList(UserIndex) | ||
| Dim i As Long | ||
|
|
@@ -401,7 +401,7 @@ HerreroQuitarMateriales_Err: | |
| Call TraceError(Err.Number, Err.Description, "Trabajo.HerreroQuitarMateriales", Erl) | ||
| End Sub | ||
|
|
||
| Sub CarpinteroQuitarMateriales(ByVal UserIndex As Integer, ByVal ItemIndex As Integer, ByVal Cantidad As Integer, ByVal CantidadElfica As Integer, ByVal CantidadPino As Integer) | ||
| Sub CarpinteroQuitarMateriales(ByVal UserIndex As Integer, ByVal ItemIndex As Integer, ByVal Cantidad As Long, ByVal CantidadElfica As Integer, ByVal CantidadPino As Integer) | ||
| On Error GoTo CarpinteroQuitarMateriales_Err | ||
| If ObjData(ItemIndex).Madera > 0 Then Call QuitarObjetos(Wood, Cantidad, UserIndex) | ||
| If ObjData(ItemIndex).MaderaElfica > 0 Then Call QuitarObjetos(ElvenWood, CantidadElfica, UserIndex) | ||
|
|
@@ -927,7 +927,7 @@ Public Sub HerreroConstruirItem(ByVal UserIndex As Integer, ByVal ItemIndex As I | |
| Call SendData(SendTarget.ToPCAliveArea, UserIndex, PrepareMessagePlayWave(MARTILLOHERRERO, UserList(UserIndex).pos.x, UserList(UserIndex).pos.y)) | ||
| UserList(UserIndex).Counters.Trabajando = UserList(UserIndex).Counters.Trabajando + 1 | ||
| If IsFeatureEnabled("gain_exp_while_working") Then | ||
| Call GiveExpWhileWorking(UserIndex, UserList(UserIndex).invent.EquippedWorkingToolObjIndex, e_JobsTypes.Blacksmith) | ||
| Call GiveExpWhileWorking(UserIndex, MiObj.ObjIndex, e_JobsTypes.Blacksmith, MiObj.Amount) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactor this to pass |
||
| Call WriteUpdateExp(UserIndex) | ||
| Call CheckUserLevel(UserIndex) | ||
| End If | ||
|
|
@@ -1035,7 +1035,7 @@ Public Sub CarpinteroConstruirItem(ByVal UserIndex As Integer, ByVal ItemIndex A | |
| End If | ||
| Call SubirSkill(UserIndex, e_Skill.Carpinteria) | ||
| If IsFeatureEnabled("gain_exp_while_working") Then | ||
| Call GiveExpWhileWorking(UserIndex, UserList(UserIndex).invent.EquippedWorkingToolObjIndex, e_JobsTypes.Carpenter) | ||
| Call GiveExpWhileWorking(UserIndex, MiObj.ObjIndex, e_JobsTypes.Carpenter, MiObj.Amount) | ||
| Call WriteUpdateExp(UserIndex) | ||
| Call CheckUserLevel(UserIndex) | ||
| End If | ||
|
|
@@ -1093,7 +1093,7 @@ Public Sub AlquimistaConstruirItem(ByVal UserIndex As Integer, ByVal ItemIndex A | |
| Call SubirSkill(UserIndex, e_Skill.Alquimia) | ||
| Call UpdateUserInv(True, UserIndex, 0) | ||
| If IsFeatureEnabled("gain_exp_while_working") Then | ||
| Call GiveExpWhileWorking(UserIndex, UserList(UserIndex).invent.EquippedWorkingToolObjIndex, e_JobsTypes.Alchemist) | ||
| Call GiveExpWhileWorking(UserIndex, UserList(UserIndex).invent.EquippedWorkingToolObjIndex, e_JobsTypes.Alchemist, MiObj.Amount) | ||
| Call WriteUpdateExp(UserIndex) | ||
| Call CheckUserLevel(UserIndex) | ||
| End If | ||
|
|
@@ -2051,26 +2051,62 @@ Public Function GetExtractResourceForLevel(ByVal level As Integer) As Integer | |
| GetExtractResourceForLevel = RandomNumber(lower, upper) | ||
| End Function | ||
|
|
||
| Public Function GiveExpWhileWorking(ByVal UserIndex As Integer, ByVal ItemIndex As Integer, ByVal JobType As Byte) | ||
| Public Function GiveExpWhileWorking(ByVal UserIndex As Integer, ByVal ItemIndex As Integer, ByVal JobType As Byte, Optional ByVal Quantity As Integer = 1) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactor this to
You changed: Public Function GiveExpWhileWorking(... Optional ByVal Quantity As Integer = 1) But you’re passing MiObj.Amount which is now Long. In VB6 this will coerce, but it’s still wrong-footed and can overflow if Amount > 32767. Action: make it: Optional ByVal Quantity As Long = 1 and keep tmpExp As Long (good change). |
||
| On Error GoTo GiveExpWhileWorking_Err: | ||
| Dim tmpExp As Byte | ||
| Dim tmpExp As Long | ||
| Select Case JobType | ||
| Case e_JobsTypes.Miner | ||
| tmpExp = SvrConfig.GetValue("MiningExp") | ||
| Case e_JobsTypes.Woodcutter | ||
| tmpExp = SvrConfig.GetValue("FellingExp") | ||
| Case e_JobsTypes.Blacksmith | ||
| tmpExp = SvrConfig.GetValue("ForgingExp") | ||
| If ObjData(ItemIndex).LingH > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).LingH * SvrConfig.GetValue("IronForgingExp") * Quantity | ||
| End If | ||
| If ObjData(ItemIndex).LingP > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).LingP * SvrConfig.GetValue("SilverForgingExp") * Quantity | ||
| End If | ||
| If ObjData(ItemIndex).LingO > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).LingO * SvrConfig.GetValue("GoldForgingExp") * Quantity | ||
| End If | ||
| If ObjData(ItemIndex).Blodium > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).Blodium * SvrConfig.GetValue("BlodiumForgingExp") * Quantity | ||
| End If | ||
| Case e_JobsTypes.Carpenter | ||
| tmpExp = SvrConfig.GetValue("CarpentryExp") | ||
| If ObjData(ItemIndex).Madera > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).Madera * SvrConfig.GetValue("WoodCarpentryExp") * Quantity | ||
| End If | ||
| If ObjData(ItemIndex).MaderaElfica > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).MaderaElfica * SvrConfig.GetValue("ElvenWoodCarpentryExp") * Quantity | ||
| End If | ||
| Case e_JobsTypes.Woodcutter | ||
| tmpExp = SvrConfig.GetValue("FellingExp") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You currently have Woodcutter twice: Case e_JobsTypes.Woodcutter Not harmful (first match wins), but it’s dead/duplicate code and can hide future edits. Action: remove the duplicate case. |
||
| Case e_JobsTypes.Fisherman | ||
| If ObjData(ItemIndex).Power >= 2 Then | ||
| tmpExp = SvrConfig.GetValue("FishingExp") | ||
| End If | ||
| Case e_JobsTypes.Alchemist | ||
| tmpExp = SvrConfig.GetValue("MixingExp") | ||
| If ObjData(ItemIndex).FlorRoja > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).FlorRoja * SvrConfig.GetValue("MixingExp") | ||
| End If | ||
| If ObjData(ItemIndex).FlorOceano > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).FlorRoja * SvrConfig.GetValue("MixingExp") | ||
| End If | ||
| If ObjData(ItemIndex).ColaDeZorro > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).FlorRoja * SvrConfig.GetValue("MixingExp") | ||
| End If | ||
| If ObjData(ItemIndex).Tuna > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).FlorRoja * SvrConfig.GetValue("MixingExp") | ||
| End If | ||
| If ObjData(ItemIndex).HongoDeLuz > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).FlorRoja * SvrConfig.GetValue("MixingExp") | ||
| End If | ||
| If ObjData(ItemIndex).SemillasPros > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).FlorRoja * SvrConfig.GetValue("MixingExp") | ||
| End If | ||
| If ObjData(ItemIndex).Cala > 0 Then | ||
| tmpExp = tmpExp + ObjData(ItemIndex).FlorRoja * SvrConfig.GetValue("MixingExp") | ||
| End If | ||
| Case Else | ||
|
Comment on lines
+2089
to
2110
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You added: If ObjData(ItemIndex).FlorOceano > 0 Then …and similarly for ColaDeZorro, Tuna, HongoDeLuz, SemillasPros, Cala: they all add FlorRoja. That’s almost certainly a bug. Fix: each branch should multiply its own field: FlorOceano branch uses ObjData(ItemIndex).FlorOceano ColaDeZorro uses ObjData(ItemIndex).ColaDeZorro etc. Also: you don’t apply * Quantity in the Alchemist case (you do for forging/carpentry). If bulk crafting is intended to scale EXP, you likely want * Quantity for Alchemist too. |
||
| tmpExp = SvrConfig.GetValue("ElseExp") | ||
| End Select | ||
|
|
||
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.
Changing a UDT field type (and even just its name, though VB is case-insensitive) changes the binary layout of t_UserOBJ.
That can break anything that:
Get/Put’s inventory structs to disk (binary saves, charfiles, etc.)
Sends inventory structs over the network (packet layouts)
Copies memory blobs around (e.g., CopyMemory, RtlMoveMemory, custom serialization)
Even if you never serialize t_UserOBJ, a UDT size change can still break code that assumes offsets.
Action: search for any Get #, Put #, Len(t_UserOBJ), CopyMemory, or packet building that writes t_UserOBJ raw. If any exist, you need a migration/compat layer or keep stored format as Integer and clamp.
Also: Inventory stack sizes being Long is reasonable, but you should enforce caps anyway (anti-dupe / overflow / economy).